-
Notifications
You must be signed in to change notification settings - Fork 188
Implement RFE #6058 display cost of orderd parts #6674
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
Implement RFE #6058 display cost of orderd parts #6674
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6674 +/- ##
============================================
+ Coverage 11.40% 11.46% +0.06%
- Complexity 6482 6504 +22
============================================
Files 1095 1097 +2
Lines 141048 140690 -358
Branches 21884 21807 -77
============================================
+ Hits 16082 16127 +45
+ Misses 123308 122893 -415
- Partials 1658 1670 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
in procurementTable
- use NegativeHexColor if total costs exceed funds
998b772
to
48e999d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, great work, most of my feedback comes down to typos or stylistic stuff. :)
I've marked the instances where you're missing copyright notices, can you also check the other classes you modified to make sure they don't need their copyright notice updated to include current year?
MekHQ/src/mekhq/gui/dialog/nagDialogs/nagLogic/UnableToAffordShoppingListNag.java
Show resolved
Hide resolved
...Q/unittests/mekhq/gui/dialog/nagDialogs/nagLogic/UnableToAffordShoppingListNagLogicTest.java
Show resolved
Hide resolved
@BeforeEach | ||
public void beforeEach() { | ||
mockCampaign = mock(Campaign.class); | ||
mockCampaignOptions = mock(CampaignOptions.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to use a real object of CampaignOptions, rather than mocking it. Could be wrong there, but it's worth checking if you haven't already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried getting a working Campaign Object. but the shopping list is very complex, as it needs
a location, and many different options set, or adding items to the sopping list, or creating an IAcquisitionWork Object will throw an NPE or other exception.
If there is a better way to set up the Campaign, CampaignOptions i would like to know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, campaign I know you can't use a real object without initializing half the client. I had hoped CO wasn't the same, but alas I see that isn't the case - at time of writing I couldn't remember.
Unfortunately, I don't think we have a testable campaign object outside of mock right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work :)
There was a problem hiding this 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 implements the feature to display the total cost of parts in procurement and triggers an insufficient funds nag when necessary. Key changes include:
- Refactoring total cost calculations by adding a getTotalBuyCost() method in IAcquisitionWork and ShoppingList.
- Updating UI components (ProcurementTableModel and CommandCenterTab) to display total cost correctly and extending the nag dialog functionality.
- Adding unit tests to validate the new cost calculation and nag triggering logic.
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
MekHQ/unittests/mekhq/gui/dialog/nagDialogs/nagLogic/UnableToAffordShoppingListNagLogicTest.java | Added tests covering the shopping list affordability logic. |
MekHQ/unittests/mekhq/campaign/parts/TotalBuyCostTest.java | Added tests ensuring the correct calculation of total buy cost. |
MekHQ/src/mekhq/gui/model/ProcurementTableModel.java | Updated to display total cost using the new getTotalBuyCost() method. |
MekHQ/src/mekhq/gui/dialog/nagDialogs/nagLogic/UnableToAffordShoppingListNag.java | Implements the affordability check for shopping list items. |
MekHQ/src/mekhq/gui/dialog/nagDialogs/UnableToAffordShoppingListNagDialog.java | Added new dialog to alert users when funds are insufficient. |
MekHQ/src/mekhq/gui/dialog/NagController.java | Integrated the new shopping list nag trigger into daily checks. |
MekHQ/src/mekhq/gui/dialog/MHQOptionsDialog.java | Extended options to include the shopping list nag preference. |
MekHQ/src/mekhq/gui/CommandCenterTab.java | Added UI elements for displaying total cost and updating it upon quantity changes. |
MekHQ/src/mekhq/campaign/work/IAcquisitionWork.java | Provided a default implementation for calculating total buy cost. |
MekHQ/src/mekhq/campaign/market/ShoppingList.java | Implemented the aggregation of total buy cost from shopping list entries. |
MekHQ/src/mekhq/MHQConstants.java | Introduced a constant for the new shopping list nag. |
Files not reviewed (3)
- MekHQ/resources/mekhq/resources/CampaignGUI.properties: Language not supported
- MekHQ/resources/mekhq/resources/GUI.properties: Language not supported
- MekHQ/resources/mekhq/resources/NagDialogs.properties: Language not supported
Implement alternative to RFE #6058
I did not implement add the "Total Cost" to the "Parts in Use" window
as that is already overcrounded and needs to be redone.
display total cost of Parts in procurement in Comand Certer Tab
NAG if total cost > Funds
refactor calculation of totalBuyCost for IAcquisitionWork
ground work to implement "Total Cost" in "Parts in Use" window
fixed sorting the procurement table by "Total Cost"
extracted in PR Fix 2 unreported problems in procurement table #6690
Update Total Cost Cell on inreasing decreasing part quantity in Table
extracted in PR Fix 2 unreported problems in procurement table #6690
Added UnitTests