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.

Sign up for GitHub

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

Jump to bottom

Inactivating vendors #4959

Merged
merged 13 commits into from
Feb 14, 2025
Merged

Inactivating vendors #4959

merged 13 commits into from
Feb 14, 2025

Conversation

jorgedjr21
Copy link
Contributor

@jorgedjr21 jorgedjr21 commented Jan 29, 2025 •
edited
Loading

Partial #4949

This PR allows only the deactivation/activation of a vendor

Description

  • Allows a vendor to be deactivated and reactivated
  • Adds new active field on vendor table
  • filter vendors by status (active/inactive)

Type of change

  • New feature for vendors: Adds the ability to activate and deactivate vendors. Deactivated vendors will not show on purchases form.
  • Adds filter for vendors active/inactive status

How Has This Been Tested?

Spects were added on:

  • spec/models/vendor_spec.rb
  • spec/system/purchase_system_spec.rb
  • spec/system/vendor_system_spec.rb

Also did some manual tests while developing the feature.

Screenshots

image
image
image
image

@cielf
Copy link
Collaborator

cielf commented Jan 30, 2025

Hey @jorgedjr21 for future PRs, if it doesn't finish resolving the issue, we say "Partial" rather than "Resolves" (I changed it above)
If we have "Resolves", the workflow on github closes the issue when the PR is merged.

@jorgedjr21 jorgedjr21 changed the title initial hability to deactivate the vendor Inactivating vendors Feb 1, 2025
@jorgedjr21 jorgedjr21 marked this pull request as ready for review February 1, 2025 19:02
@jorgedjr21
Copy link
Contributor Author

@cielf When you have time, can you review this PR, please?
Thanks you!

@cielf
Copy link
Collaborator

cielf commented Feb 4, 2025

Looks pretty good to me functionally. Over to @dorner for technical and automated testing comments.

@cielf cielf requested a review from dorner February 4, 2025 21:02
dorner
dorner requested changes Feb 4, 2025
View reviewed changes
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.

Thanks for your work!

app/controllers/vendors_controller.rb Outdated Show resolved Hide resolved
app/models/vendor.rb Outdated Show resolved Hide resolved
db/schema.rb Outdated Show resolved Hide resolved
spec/system/purchase_system_spec.rb Outdated Show resolved Hide resolved
spec/system/vendor_system_spec.rb Outdated Show resolved Hide resolved
spec/system/vendor_system_spec.rb Outdated Show resolved Hide resolved
@jorgedjr21 jorgedjr21 requested a review from dorner February 12, 2025 23:22
dorner
dorner approved these changes Feb 14, 2025
View reviewed changes
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.

Looks good to me!

@dorner dorner merged commit 007fad6 into rubyforgood:main Feb 14, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dorner dorner dorner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jorgedjr21 @cielf @dorner