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 stage deletion by removing unused tables #697

Merged
merged 9 commits into from
Oct 9, 2024
Merged

Conversation

kalilsn
Copy link
Member

@kalilsn kalilsn commented Oct 7, 2024

Resolves #632

High-level Explanation of PR

The unjournal stages that can't be deleted have a foreign key reference to some unused tables that were intended to track history of assignment and stage movement, but aren't used anymore. Dropping those stages will allow the IDs to be fixed.

  • Drops the action_move and action_claim tables
  • Sets submit button stage to null if its referenced stage is deleted
    • This could be improved by warning the user before deleting the stage, but I had trouble (async/await issues) making that work. Draft PR with my quick attempt here.

Test Plan

  • Set a stage on a submit button, then try deleting that stage. It should succeed
  • No real way to test the other changes but I'll delete that pesky stage in prod after this is deployed

These tables aren't used by our code anymore and were originally intended
to keep a history of claims and moves on a pub. Since we're not doing that
currently I have removed both tables to avoid any confusion
@@ -154,9 +153,6 @@ export default async function Page({
) : null}
</div>
</div>
<div>
<MembersAvatars pub={pub} />
</div>
Copy link
Member Author

Choose a reason for hiding this comment

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

This wasn't rendering anything on most pubs, and it's confusing when it is rendered. See the "Assigned To" section on this pub
image

@kalilsn kalilsn requested review from tefkah and allisonking and removed request for tefkah October 7, 2024 18:32
Copy link
Contributor

@tefkah tefkah left a comment

Choose a reason for hiding this comment

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

Looks great! I'll have a look at the draft PR to see if that's feasible, but otherwise great work!

Copy link
Contributor

Choose a reason for hiding this comment

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

Very good changes!

@kalilsn kalilsn enabled auto-merge (squash) October 9, 2024 19:10
@kalilsn kalilsn merged commit 5d75b1f into main Oct 9, 2024
7 checks passed
@kalilsn kalilsn deleted the kalilsn/stage-deletion branch October 9, 2024 19:13
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.

Unjournal: delete stage
2 participants