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

warn about deleting other peoples' tables #226

Closed
MichaelCoulter opened this issue Apr 21, 2022 · 10 comments · Fixed by #711
Closed

warn about deleting other peoples' tables #226

MichaelCoulter opened this issue Apr 21, 2022 · 10 comments · Fixed by #711
Assignees
Labels
enhancement New feature or request

Comments

@MichaelCoulter
Copy link
Collaborator

it would be great to display the team name associated with the tables when you are trying to delete tables. this might help people accidentally delete other peoples' tables. for example, this can be a problem when a set of parameters is used by multiple people.

@dpeg22 dpeg22 added the enhancement New feature or request label Apr 22, 2022
@dpeg22
Copy link
Collaborator

dpeg22 commented Apr 22, 2022

This could look similar to the extended delete method SpikeSorting.

@lfrank
Copy link
Contributor

lfrank commented Jun 26, 2022

This would be great, but has to be done by modifying every table to include a custom delete method, and unfortunately the custom delete methods don't get called when deleting downstream tables.

An easier current solution would be for people to make their own tables (e.g. mcoulter_spikesorting) and add their sorts / other entries to those tables, as only the owner and potentially other specific team members can delete tables like this.

@MichaelCoulter
Copy link
Collaborator Author

@khl02007 just reminding you about this issue

@edeno
Copy link
Collaborator

edeno commented May 25, 2023

@CBroz1 If you have any thoughts about this issue in the future (doesn't have to be now), that would also be great.

@CBroz1
Copy link
Member

CBroz1 commented May 25, 2023

A couple things come to mind, each with pros and cons...

  1. Forking datajoint to inject a custom delete protocol.
  • Pro: custom delete behavior with minimal spyglass edits
  • Con: huge maintenance overhead with merge conflicts, and end-user custom installs
  1. Revising MySQL permissions around prefixes. For example, only admin may declare/delete the prefix for stable pipelines and users may only declare/delete on their username prefix.
  • Pro: database security with permission escalation, and making notions of 'finished' vs 'in-flux' tables explicit
  • Con: database migration
  1. Adding a helper function in utils for a cautious delete that spits out upstream info before confirming a delete
  • Pro: Easy to implement
  • Con: unenforceable

If I understand the status quo correctly, I would be very much in favor of 2

@edeno
Copy link
Collaborator

edeno commented May 25, 2023

Given the hassle of database migration, I think we should at least try the third option first and see if it works well enough. It might not be the long term solution, but worth a shot in the short term. @CBroz1 I will assign this issue to you and then at some time in the next month could you try submitting a PR for it?

@CBroz1
Copy link
Member

CBroz1 commented May 25, 2023

I'll clarify that I'm not certain migrating would be required. Renaming in place might be feasible, but there may be other issues.

Certainly, happy to develop a helper function to this effect

@MichaelCoulter
Copy link
Collaborator Author

thanks chris. it would be great for the cautious delete statement to display the number of rows potentially deleted in each table grouped by "team_name" for the tables that have that key.

@CBroz1
Copy link
Member

CBroz1 commented Dec 2, 2023

Looking into this more, it looks like each Session is associated with an experimenter, who can be on one or more teams. I can return all of the teams associated with the individual linked to the data, but that seems cumbersome

@edeno What do I need in order to have permission to delete? Do I need to (a) be the experimenter assoc. w/the session, or (b) be on at least one team with the session experimenter? Is (a) the default, but (b) requires a force flag? Or is (b) the default and delete any requires the extra flag?

I think the best possible implementation of this would require some surgery to link a session not to an individual but a team, but that would take some doing

@CBroz1
Copy link
Member

CBroz1 commented Dec 15, 2023

🥳 🎉 Progress #711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants