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

Fix relation lists (regression #359) #719

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

Quicksaver
Copy link
Contributor

- Summary

Fixes #359 (regression)

- Test plan

  • Not sure exactly when the regression happened, but the exact behavior described in Only partial relation metadata available on list previews #359 could be seen again for a few releases now.
  • Tested manually in exactly the same way as in #359 - Handle metadata for all children of a list field #360: adding, removing and updating any child of the list works, switching back and forth between pages and reloading the page has the correct metadata now.
  • Tested also simple string relation fields, work as expected with the new code as well.
  • Nothing to be changed in the docs, they don't really cover relation widgets much anyway.
  • This definitely needs automated tests to prevent further regressions, but there's absolutely nothing covering relation fields already (that I could find), and I'm not familiar enough with the test platform or existing tests to jump in with no base.
  • I'm having trouble running lint, it fails monumentally (~300k errors) even on the current master, so I'm skipping that.

- Description for the changelog

(Fix regression) Handle metadata for all children of a list field

- A picture of a cute animal (not mandatory but encouraged)
cute-baby-chickens-wearing-knit-hats1

@erquhart
Copy link
Contributor

erquhart commented Oct 23, 2017

@Quicksaver thanks for this! Looks like relation previews are totally broken, both in the latest release and in this PR - this fix would have to go on top of a fix for the basic issue of relation previews not loading, mostly because it's difficult to ensure this fix doesn't cause any regressions as it stands. Would you be able to take a look?

  1. Check out the kitchen sink entry in the deploy preview for this PR
  2. Note that the preview for the Related Post field (first field in the entry) is missing
  3. Change the value to the current post (search for "TOML" and reselect)
  4. Note the preview is still missing
  5. Change the value to a different post (search for "JSON" and select the post that pops up)
  6. The preview is now present

If you can't look into this it's totally fine, someone will just need to before we can move forward with this PR.

@Quicksaver
Copy link
Contributor Author

@erquhart sorry for not responding sooner. I did take a look at the kitchen sink example but I get a slightly different behavior than you. On step 4 I get the correct preview already, tried with the very latest master and with my patch, same behavior. But it is also missing when first loading the page for me.

fieldsMetaData is empty in the RelationKitchenSinkPostPreview component when first loaded apparently. I'll try to take a more thorough look over the next week to see if I can track it down, but if anyone wants to before I can please go ahead. 😃

@erquhart
Copy link
Contributor

Some of this seems to have been (inadvertently) resolved in the UI overhaul, at least the base issue of the relation widget not working. Will follow up after UI overhaul is merged.

@verythorough
Copy link
Contributor

Deploy preview ready!

Built with commit d76c90f

https://deploy-preview-719--netlify-cms-www.netlify.com

@verythorough
Copy link
Contributor

Deploy preview ready!

Built with commit d76c90f

https://deploy-preview-719--cms-demo.netlify.com

@Quicksaver
Copy link
Contributor Author

@erquhart I rebased the commit and found out perhaps a better way of fixing the issue. Not sure if this was possible before or only now after the UI overhaul. But I've tested locally in the same way as before, all seems fine.

I should note that the relation widget in the kitchen sink entry doesn't appear during the first page load for me, not sure why that is although admittedly I haven't investigated too deeply. All of my custom relation widgets work just fine though, so that is likely a problem with the demo preview widget itself rather than with the CMS.

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for seeing this through @Quicksaver!

@erquhart erquhart merged commit 3c21a6a into decaporg:master Jan 23, 2018
@Quicksaver
Copy link
Contributor Author

@erquhart it was my pleasure, thanks for merging it in. 😃

@Quicksaver Quicksaver deleted the bug/relation_lists branch January 24, 2018 09:39
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.

Only partial relation metadata available on list previews
3 participants