[Rails] Don't pass orphaned Stories (e.g. no Places or Speakers) to React - Githubissues

Terrastories / terrastories

Terrastories is a geostorytelling application for mapping, managing and sharing place-based stories.

https://terrastories.app

MIT License

316 stars 157 forks source link

[Rails] Don't pass orphaned Stories (e.g. no Places or Speakers) to React #956

Open rudokemper opened 1 year ago

rudokemper commented 1 year ago

Currently, it's not possible to create a Story without selecting at least one Place and at least one Speaker.

However, it is possible to delete Places and Speakers, which if done can leave a Story "orphaned" (e.g. it exists but has no Places, or Speakers, or both).

It's fine to keep it this way on the Rails side for now -- we need to think about our data structures more -- but we should handle this on the React side.

What is happening now is that the stories will still show up in StoryList. Only, if there are no Speakers, nothing will show at the top, and (worse) if there are no Places, clicking on the card does nothing.

Let's filter out any stories that have 0 Places or 0 Speakers on the React front end. This also means we should filter out any markers on the map that ONLY have stories that have been filtered out.

Acceptance criteria:

[ ] If I create a Story, a Place, and a Speaker, and then I delete the Place OR the Speaker (or both), the Story should continue to exist in Rails but it should not be passed to React (e.g. it doesn't show up in StoryList component).
[ ] If I create a Story, a Place, and a Speaker, and I delete the Speaker, the Story should not be passed to React, and the Place should also not be passed to React (IF is the only Story associated with it; it should continue to appear on the map if it has other Stories with the minimum validation of >1 Place and >1 Speaker).

ice1080 commented 11 months ago

I can take this one!

ice1080 commented 11 months ago

Question on this one, assuming for @rudokemper, I'll be filtering out the orphaned stories from dashboard/stories_controller, but should do the same to api/stories_controller? I'm less familiar with what the api module is used for

rudokemper commented 11 months ago

Great question. That is actually used by a new Terrastories React view at https://github.com/terrastories/explore-terrastories, which we still need to document properly. I think it'd be good to filter it out from there as well.

ice1080 commented 11 months ago

Thanks for the follow up! Regarding this issue, I've got the changes locally but wanted to make sure there are no tests around the pages or dashboard controllers (except Manage Community). Is this correct or am I not seeing them?

A related note, here is a potential way to avoid orphaned parents in this situation. Our use case would be slightly more complex since there are two children to check in the parent, but could still work. https://stackoverflow.com/questions/5866378/delete-orphaned-parent

rudokemper commented 11 months ago

Thank you! I don't believe we have any tests for the pages or controllers. That would be a welcome addition, but I'm also happy to test your work locally for this.

If I'm understanding that article correctly, the suggestion would be to delete any parent if there are no more children left, right? That would not be desirable in our use case; a user might be drafting content and we want to be very intentional about giving them complete control over their actions. Some of our users are not very technical and a piece of content disappearing on them (because it was deleted due to orphanage) could be confusing.

ice1080 commented 11 months ago

I'm new to rails but not to unit testing, so I'll see if I can put together a couple tests around these.

What you mentioned makes sense in terms of not wanting content to disappear because it's deleted, however wouldn't that pretty much be the case with acceptance 1? If they delete the last speaker or place attached to a story, then that story no longer shows up in the list of stories that can be edited right? So they won't be able to add a speaker/place back? Or did I misinterpret this one?

rudokemper commented 11 months ago

Acceptance criteria #1 means that the story should continue to exist in the db and is accessible for editing or viewing in Rails, but it should no longer show in the StoryList component on React (on the front end, e.g. /home/ route)

ice1080 commented 11 months ago

Got it! I'll update my work; it might be that both acc 1 and 2 are complete by the work for 2.

ice1080 commented 11 months ago

This one is ready for review @rudokemper! #971