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 stickyTags to include params api url #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janmuran
Copy link

@janmuran janmuran commented Sep 9, 2022

ajax requests url parameter include does not contain a stickyTags

/api/discussions?include=user,lastPostedUser,tags,tags.parent,firstPost&filter[tag]=slug&sort=&page[offset]=0

that cause error in StickiestModal.js

TypeError: i.stickyTags().filter is not a function

on line 23: ( const stickyTags = Stream(discussion.stickyTags().filter((tag) => discussionTags.indexOf(tag) > -1) || []);)

@the-turk
Copy link
Owner

the-turk commented Sep 9, 2022

thanks for the PR! But these lines should already include the necessary relationship between discussion and its sticky tags:

https://github.com/the-turk/flarum-stickiest/blob/master/extend.php#L60

https://github.com/the-turk/flarum-stickiest/blob/master/js/src/forum/index.js#L29

when exactly did you have that error?

@janmuran
Copy link
Author

janmuran commented Sep 9, 2022

Relation to stickyTags works fine, but when ajax request has include parameter in url (...discussions?include=user,lastPostedUser,...) stickyTags is filtered in:

AbstractSerializerController on line: 128:

        $element = $this->createElement($data, $serializer)
            ->with($this->extractInclude($request))  - this remove relation to stickyTags on output
           ->fields($this->extractFields($request));

        $document->setData($element);

        return new JsonApiResponse($document);

When loading page for the first time, everything works fine. flarum-json-payload has data with relation to stickyTags. After is clicked on menu, ajax request called url like /api/discussions?include=...., when stickyTags is not in include, response is without stickyTags. When url is without include response is with all relations

@the-turk
Copy link
Owner

the-turk commented Sep 9, 2022

That line in AbstractSerializer is not removing, but including the relationships. I still believe that whatever this relation issue is, can be fixed without touching the javascript. Could you provide me the steps to reproduce this error?

@janmuran
Copy link
Author

I try add relation in beforeSerializationCallbacks, but createElement with include from request remove stickyTags

Steps with error:

  1. open forum - discussions with sticky tags are ok
  2. click on menu child item
  3. click on submenu create ajax request with include parameter in query

/api/discussions?include=user,lastPostedUser,tags,tags.parent,firstPost&filter[tag]=halloween-2021&sort=&page[offset]=0

  • there are no stickyTags relations
  1. add stickyTags to query

/api/discussions?include=user,lastPostedUser,tags,stickyTags,tags.parent,firstPost&filter[tag]=halloween-2021&sort=&page[offset]=0

  • stickyTags are in response

I check controller:
ListDiscussionsController
line: 96
$include = array_merge($this->extractInclude($request), ['state']); - this line cause, that stickyTags relation is not in $includes

line: 128 $this->loadRelations($results, $include, $request); - relations should be loaded here, but $include is without stickyTags

@FinKiin
Copy link

FinKiin commented Mar 5, 2023

janmuran,you are my hero !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants