Skip to content

includes : disable static checks when using polymorphic relations - #13

Closed
ThomasSevestre wants to merge 3 commits into
jsonapi-serializer:devfrom
ThomasSevestre:polymorphic_includes
Closed

includes : disable static checks when using polymorphic relations#13
ThomasSevestre wants to merge 3 commits into
jsonapi-serializer:devfrom
ThomasSevestre:polymorphic_includes

Conversation

@ThomasSevestre

@ThomasSevestre ThomasSevestre commented Oct 4, 2019

Copy link
Copy Markdown

when including whatever.polymorphic_relation.something_else, original code complains about something_else not being compatible with whatever

@stas

stas commented Oct 5, 2019

Copy link
Copy Markdown
Collaborator

@ThomasSevestre would you be kind to provide a test case to replicate this scenario pretty please?

@ThomasSevestre

Copy link
Copy Markdown
Author

any feedback ?

@kpheasey
kpheasey changed the base branch from master to dev October 15, 2019 18:46

@mocerinoj mocerinoj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.. having the same issue

@stas stas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasSevestre apologies for the late reply. Could you rebase this PR so we can merge it.

Thank you!

list = List.new
list.id = 1
list.items = [checklist_item, car]
expect(-> {ListSerializer.new(list, include: ["items.list"])}).not_to raise_error

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christophersansone

Copy link
Copy Markdown
Contributor

Just an FYI, I saw this as well when building #32 and fixed it there too. Same logic, just break if the serializer may change.

@stas

stas commented Feb 15, 2020

Copy link
Copy Markdown
Collaborator

Superseded by #32

@stas stas closed this Feb 15, 2020
@stas

stas commented Feb 15, 2020

Copy link
Copy Markdown
Collaborator

Thanks again @ThomasSevestre

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.

4 participants