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

Cleanup factories and annotate them, drop partner users table #4536

Merged
merged 7 commits into from
Aug 1, 2024

Conversation

jimmyli97
Copy link
Contributor

@jimmyli97 jimmyli97 commented Jul 16, 2024

Description

Clean up factories and annotate missing annotations

  • rename factories so that annotate gem picks them up
  • partners_user / partner_user is a duplicated factory, and also belongs under the user factory not in its own module since it is part of the users table
  • partner_users table can be dropped since it was merged in commit 838d511

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

How Has This Been Tested?

Passes test suite

@jimmyli97 jimmyli97 marked this pull request as draft July 16, 2024 20:40
@jimmyli97 jimmyli97 force-pushed the cleanup-annotate-factories branch 4 times, most recently from e53d10f to f431a68 Compare July 16, 2024 21:19
@jimmyli97 jimmyli97 marked this pull request as ready for review July 16, 2024 21:26
@jimmyli97 jimmyli97 marked this pull request as draft July 17, 2024 22:56
@jimmyli97 jimmyli97 force-pushed the cleanup-annotate-factories branch from f431a68 to 1d686be Compare July 18, 2024 01:21
@jimmyli97 jimmyli97 marked this pull request as ready for review July 18, 2024 01:26
@jimmyli97 jimmyli97 changed the title Cleanup factories and annotate them Cleanup factories and annotate them, drop partner users table Jul 18, 2024
Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Overall looks good, I had one question.

@@ -51,6 +51,22 @@
end
end

factory :partners_user do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this renamed?

Copy link
Contributor Author

@jimmyli97 jimmyli97 Jul 20, 2024

Choose a reason for hiding this comment

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

There were two identical factories, one called :partner_user defined in factories/partner_user.rb and one called :partners_user defined in factories/partners/user.rb

The codebase used both so I just did a coin flip and picked one to rename all of them to

@jimmyli97 jimmyli97 requested a review from dorner July 20, 2024 01:27
Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Sorry - can you switch your coin flip and go back to partner_user? :) 

Also, there is now a conflict on the schema file that needs to be resolved.

@jimmyli97 jimmyli97 requested a review from dorner July 31, 2024 03:10
@dorner
Copy link
Collaborator

dorner commented Aug 1, 2024

Nice, thanks!

@dorner dorner merged commit 4cd964d into rubyforgood:main Aug 1, 2024
19 checks passed
@jimmyli97 jimmyli97 deleted the cleanup-annotate-factories branch August 1, 2024 12:03
Copy link
Contributor

@jimmyli97: Your PR Cleanup factories and annotate them, drop partner users table is part of today's Human Essentials production release: 2024.08.11.
Thank you very much for your contribution!

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.

2 participants