Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add back sources relation in the discussion's API serializer, while still respecting the permissions #13

Open
n-peugnet opened this issue Feb 2, 2023 · 1 comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@n-peugnet
Copy link
Member

n-peugnet commented Feb 2, 2023

This will should lower the network and client usage.

If I understand correctly, something similar is done in flarum/mentions in this file: https://github.com/flarum/mentions/blob/v1.6.3/src/FilterVisiblePosts.php

Which is of course wired in extend.php:

    (new Extend\ApiController(Controller\AbstractSerializeController::class))
        ->prepareDataForSerialization(FilterVisiblePosts::class),
@n-peugnet n-peugnet added the enhancement New feature or request label Feb 2, 2023
@n-peugnet n-peugnet added this to the v1.1.0 milestone Feb 2, 2023
@n-peugnet n-peugnet added the help wanted Extra attention is needed label Feb 3, 2023
@n-peugnet
Copy link
Member Author

It definitely works correctly for mentions, so this may be a good example how to implement this in this extension, since the functionality is quite similar. However, I think there is one problem with current schema of discussion_reference - currently it tracks discussion-to-discussion relations. But for including data in payload it makes sense to store information also about post with link to discussion and generated EventPost. In this way you can include info about linked discussions using PostSerializer, so payload will contain info only about discussion needed to handle current page. I you useDiscussionSerializer, payload will contain all linked discussion, even if none of them is visible on current page - it could be a real problem for large discussions with lots of references.

Originally posted by @rob006 in #20 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant