Skip to content

⚠️ Remove create user roles migration #83

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

Merged
merged 1 commit into from
Mar 19, 2025
Merged

Conversation

lorenyu
Copy link
Contributor

@lorenyu lorenyu commented Mar 14, 2025

Ticket

n/a

Changes

see title

Context for reviewers

The commit 5e2a120 removes the user roles from the application code but not from the database. This change removes it from migrations. This change was separated out since removing an already existing migration is not backwards compatible. Since this is a template it makes sense to do it this way rather than add an extra migration that removes the user_roles table.

Migration notes

When updating the template in this version, revert the changes in these files:

  • <APP_NAME>/db/migrate/20240410213056_create_user_roles.rb (deleted in this change)
  • <APP_NAME>/db/schema.rb (user_tables removed)

Deployment notes

After merging we'll need to:

  • Recreate the schema in platform-test app-rails
  • Recreate the schema in pfml-starter-kit-app demo
  • Revert the change in pfml-starter-kit-app paidleave

Testing

see navapbc/platform-test#193

@lorenyu lorenyu requested a review from Copilot March 14, 2025 20:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the user roles migration and the corresponding user_roles table definition from the database schema as part of cleaning up legacy code.

  • Removed the create_table block for "user_roles" and its foreign key from db/schema.rb.
  • Deleted the migration file "20240410213056_create_user_roles.rb" to avoid backward compatibility issues.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
template/{{app_name}}/db/schema.rb Removed user_roles table and its associated foreign key.
template/{{app_name}}/db/migrate/20240410213056_create_user_roles.rb Deleted the migration file for creating user_roles.

@lorenyu
Copy link
Contributor Author

lorenyu commented Mar 14, 2025

@doshitan food for thought, thinking through the migration notes here, I wonder if in the future it'd be valuable to be able to mark certain template commits as ones that we'd want to ignore during an "update" command. i.e. features that we had but removed, we'd want to let existing projects keep those features, but just remove them from future clean installs.

@lorenyu lorenyu merged commit bdf0f73 into main Mar 19, 2025
3 checks passed
@lorenyu lorenyu deleted the lorenyu/rmmigration branch March 19, 2025 22:38
@lorenyu
Copy link
Contributor Author

lorenyu commented Mar 19, 2025

Post merge notes:

  • I recreated schema for platform-test app-rails and pfml-starter-kit-app demo by manually updating the configure_schema function in role_manager's manage.py to DROP SCHEMA app CASCASE
  • For paidleave app I simply rejected the change since we want to keep user roles in that app

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.

1 participant