Collection crashes after deleting one of its sketches

[hipstina]

Found a bug

Details about the bug:

• Web browser and version: FireFox
• Operating System: MacOSX Version 10.15.5
• Steps to reproduce this bug:

1. In web editor, add a sketch to any collection: File > Add to Collection.
2. In /sketches, delete the sketch that was added to collection.
3. Open the collection that was just modified in /collections...here is where the page goes to blank white page ('crashes').

Example: 'My test collection'

[welcome]

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

[slowizzm]

I was just about to file a similar issue.

Deleting a sketch that is within a collection causes the browser to freeze when trying to view the collection the sketch was in.

[dhoizner]

For anyone looking at this: this is happening due to references to a project within a collection don't get cleaned up when the project (sketch) gets deleted.

I've spent the better part of yesterday and today trying to get this working and have gotten part of the way there, but lack of familiarity with mongo and react/redux are slowing me down 😓.

Current thoughts: we should:

1. Clean up invalid references when a project is deleted
2. Clean up any existing invalid references
3. Filter out collection items that don't point to a valid project when feeding the frontend

Thoughts? I'd also be happy to pair on this with someone if anyone is interested.

[andrewn]

Hi, thanks so much for looking into this! Sorry to hear it’s slow going.

I think that you’re right, this should happen at the API level so the client just receives valid collections containing no sketches.

I think filtering deleted sketches from collections at the API level is a good solution to this bug.

The list collections controller is responsible for this from what I remember.

Is it possible to iterate through the list of returns collections, and check collection.items has returned a project? I assume it would be null if the reference hasn’t been returned, so we could filter out null projects.

Happy to help with this, just let me know.

[andrewn]

@dhoizner I thought about this a bit more and think a better approach is to:

• show that a sketch has been deleted in the collection list
• allow the user to manually delete the sketch if they want

This is a much simpler implementation and also means that sketches don't "disappear" which might be confusing for collection owners?

I opened #1480 that does this. What do you think?

[catarak]

I had a chance to look at this issue and the PR (#1480). I think the solution you (@andrewn) tried out is okay for now, and maybe a longer term solution would be to alter the data structure of sketches to store an array of collection objectids that they are in? What do y'all think?

[andrewn]

@catarak For me it depends on what the the goal of the data structure change would be?

At the moment, having collections maintain a list of sketches means that a sketch doesn’t need to be aware of collections at all, which I think helps keep the code simpler.

I’m not a mongo expert, but I think we need to avoid arrays that grow without limit. We can impose a limit to the number of sketches in Collection.items[] but it would be difficult to do for Project.in_collection[] as a popular sketch could be in any number of collections.

From the UI side, I think it’s important that deleted sketches in collections don’t disappear as if they never existed. But I guess I’m thinking of this from the point of view of User A deleting a sketch that User B has in their collection. It would be strange for it to be gone like it was never in the collection.

[catarak]

The goal of the data structure would be to make it easier to deal with deleting a sketch, so that it would be easy to iterate through all of the collections that sketch is in and update them.

I did a little research about the best way to handle Many-to-Many relationships in MongoDB, and apparently you can do one way references (like what we have now) or two way references (see Many-to-Many). I think your point about arrays that grow without limit makes sense, and maybe we could impose an arbitrary limit (like 100? 200?).

But also I think it makes sense to research if just leaving the references in the collection is okay. Do other tools have similar patterns?

[andrewn]

I think your point about arrays that grow without limit makes sense, and maybe we could impose an arbitrary limit (like 100? 200?).

Having a sketch having a maximum arbitrary limit of collections doesn't feel right to me.

We could do a query using the current data structure:
Collection.find({ 'items.project': project._id })

This is probably slower, but maybe a good trade-off if deleting a sketch doesn't have to be ultra-fast?

[catarak]

I don't think it needs to be fast! It's already a little slow if a sketch has a bunch of stuff uploaded to the asset storage. I've wondered if there is a way to run all of the deletion tasks in the background, since it's not really important for a user to wait around for them to be done. I guess this is what a message queue is for?