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

3742 low inventory report #3857

Merged
merged 7 commits into from
Nov 12, 2023

Conversation

ChaelCodes
Copy link
Contributor

@ChaelCodes ChaelCodes commented Aug 23, 2023

Resolves #3742

Description

This PR adds a low inventory report to the dashboard.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Unit test for Query Objects
  • Smoke test in local environment
  • system spec for low inventories
  • system spec for no low inventories
  • Full test run locally

Screenshots

New low inventory report:
low inventory report
there is a tooltip on hover over quantity when minimum quantity is not met: "Below minimum quantity"
screenshot of message displayed when inventory levels are fine: "Inventory is at recommended levels (minimum and recommended levels can be set on each item)

@ChaelCodes
Copy link
Contributor Author

ChaelCodes commented Aug 23, 2023

To-Do:

  • System specs
  • No data shows a message instead of an empty table
  • Notification icon for inventory below minimum levels

If there is a minimum of 0 and amount of 0, the equality check used to
return that inventory item, which we don't want. The cleanest way to
handle this is to only return if the inventory levels are lower.
@ChaelCodes ChaelCodes marked this pull request as ready for review September 28, 2023 18:34
@ChaelCodes ChaelCodes changed the title [WIP] 3742 low inventory report 3742 low inventory report Sep 28, 2023
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 like the approach - added a few questions/suggestions.

app/queries/low_inventory_query.rb Outdated Show resolved Hide resolved
let(:recommended_quantity) { 200 }

it { is_expected.to include inventory_item }
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually prefer having one big test that has all your data set up and returns a list where we check to see that all the expected data is in it and none of the unexpected data is. The only other case you need is a zero case where it returns nothing. It's much faster than creating all the data multiple times.

end
end
end
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why were these removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This donation section doesn't currently exist in the dashboard page. This PR is pointed into the dashboard branch which currently has failing specs. There's a note in the commit message. This commit is available in a separate PR, but that one hasn't been merged yet.

Copy link
Collaborator

@awwaiid awwaiid left a comment

Choose a reason for hiding this comment

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

This is great -- I'm merging these all into the main dashboard branch and will spin off any cross-PR fixes from there.

@awwaiid awwaiid merged commit 13d164a into rubyforgood:dashboard Nov 12, 2023
@awwaiid awwaiid added this to the Dashboard Update milestone Nov 12, 2023
Copy link
Contributor

@ChaelCodes: Your PR 3742 low inventory report is part of today's Human Essentials production release: 2024.05.26.
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.

None yet

3 participants