Skip to content

[RFR] isDetailView doesn't work on refernced_list entity id-field#560

Merged
jpetitcolas merged 2 commits intomarmelab:masterfrom
vasiakorobkin:go_to_detail_state_params_override
Jul 24, 2015
Merged

[RFR] isDetailView doesn't work on refernced_list entity id-field#560
jpetitcolas merged 2 commits intomarmelab:masterfrom
vasiakorobkin:go_to_detail_state_params_override

Conversation

@vasiakorobkin
Copy link
Copy Markdown

This PR is absolutely analogous to #537
This time isDetailLink() applied to referenced_list entity doesn't work because of state parameter override.
I've created a e2e-test to demonstrate it. Test fails on your master branch, and passes on my commit.

@vasiakorobkin vasiakorobkin changed the title Fix params for DetailView field of nested (referenced) entity [RFR] isDetailView doesn't work on refernced_list entity id-field Jul 15, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you just move the id at the beginning of the table? That's more conventional! :)

@jpetitcolas
Copy link
Copy Markdown
Contributor

Good for me. 👍

Can you rebase on master, as tests have been fixed yesterday to ensure everything is correct?

@jpetitcolas
Copy link
Copy Markdown
Contributor

Should fix #569.

@vasiakorobkin vasiakorobkin force-pushed the go_to_detail_state_params_override branch from 9489aca to 3fefac8 Compare July 24, 2015 08:35
@vasiakorobkin
Copy link
Copy Markdown
Author

Id field moved to the begining of the table. Branch was rebased.

@jpetitcolas
Copy link
Copy Markdown
Contributor

Your PR is still red. I don't think that's an issue with your PR. Looking at it.

jpetitcolas added a commit that referenced this pull request Jul 24, 2015
…override

[RFR] isDetailView doesn't work on refernced_list entity id-field
@jpetitcolas jpetitcolas merged commit 0becc8c into marmelab:master Jul 24, 2015
@jpetitcolas
Copy link
Copy Markdown
Contributor

Tests are correctly passing on master. Thanks for your help! :)

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.

2 participants