8000 Implement RFE #6058 display cost of orderd parts by goron111 · Pull Request #6674 · MegaMek/mekhq · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 10 commits into from
Apr 26, 2025

Conversation

goron111
Copy link
Contributor
@goron111 goron111 commented Apr 14, 2025

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.

DisplayCostOfOrderdParts1
DisplayCostOfOrderdParts2
UnableToAffordShoppingList

@goron111 goron111 marked this pull request as draft April 14, 2025 21:08
Copy link
codecov bot commented Apr 14, 2025

Codecov Report

Attention: Patch coverage is 9.72222% with 65 lines in your changes missing coverage. Please review.

Project coverage is 11.46%. Comparing base (c77b064) to head (68f6c76).
Report is 38 commits behind head on master.

Files with missing lines Patch % Lines
MekHQ/src/mekhq/gui/CommandCenterTab.java 0.00% 21 Missing ⚠️
...agDialogs/UnableToAffordShoppingListNagDialog.java 0.00% 13 Missing ⚠️
MekHQ/src/mekhq/gui/dialog/MHQOptionsDialog.java 0.00% 12 Missing ⚠️
MekHQ/src/mekhq/campaign/market/ShoppingList.java 31.25% 9 Missing and 2 partials ⚠️
...src/mekhq/gui/dialog/nagDialogs/NagController.java 0.00% 6 Missing ⚠️
...ialogs/nagLogic/UnableToAffordShoppingListNag.java 50.00% 1 Missing ⚠️
...kHQ/src/mekhq/gui/model/ProcurementTableModel.java 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@goron111 goron111 force-pushed the rfe-display-cost-of-orderd-parts branch from 998b772 to 48e999d Compare April 19, 2025 12:24
@goron111 goron111 marked this pull request as ready for review April 19, 2025 12:38
Copy link
Collaborator
@IllianiBird IllianiBird left a 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?

@BeforeEach
public void beforeEach() {
mockCampaign = mock(Campaign.class);
mockCampaignOptions = mock(CampaignOptions.class);
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

@goron111 goron111 requested a review from IllianiBird April 21, 2025 09:13
Copy link
Collaborator
@IllianiBird IllianiBird left a comment

Choose a reason for hiding this comment

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

Great work :)

@HammerGS HammerGS requested a review from Copilot April 23, 2025 02:37
Copy link
Contributor
@Copilot Copilot AI left a 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

@IllianiBird IllianiBird merged commit 6a67831 into MegaMek:master Apr 26, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0