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

Include inactive storage locations in csv #5026

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Conversation

napster235
Copy link
Contributor

Resolves #5003

Description

Type of change

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

How Has This Been Tested?

Screenshots

With checkbox On
Screenshot 2025-02-19 at 13 51 18
Screenshot 2025-02-19 at 13 51 37

With checkbox Off
Screenshot 2025-02-19 at 13 52 03

@coalest
Copy link
Collaborator

coalest commented Feb 19, 2025

I think you'll probably be asked to add or modify a test for this change. (could probably go in spec/requests/storage_locations_requests_spec.rb)

@cielf
Copy link
Collaborator

cielf commented Feb 19, 2025

Hey @napster235 -- a reminder that it's good practice to make a branch, rather than using your main.

@cielf
Copy link
Collaborator

cielf commented Feb 19, 2025

Functionality looks good. over to @dorner for tech review.

@cielf cielf requested a review from dorner February 19, 2025 15:31
@napster235
Copy link
Contributor Author

Hey @napster235 -- a reminder that it's good practice to make a branch, rather than using your main.

Ok, noted. I thought it is ok since I was on my fork.

@cielf
Copy link
Collaborator

cielf commented Feb 19, 2025

It's ok -- I have to rename it when I pull it for testing, though, and it could be a pain for you if you are ever wanting to work on two PRs that overlap in time.

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! Thank you!

@dorner dorner merged commit 07e3e4c into rubyforgood:main Feb 20, 2025
11 checks passed
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.

Inactive Storage Locations should be included in Storage Location export if inactive filter is on
4 participants