fix: Offline Currency Payout Improvements #2797
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Currency payouts would stop paying out while offline once any currency had a zero offline payout. - Changed it to keep checking additional currencies. - Issue crowbartools#2796
- Checking then unchecking the Offline Payout checkbox in the add or edit currency dialog results in the offline property being assigned to an empty string. - This conflicts with the type definition of a Currency in currency-access.ts. - So pay special attention to it potentially being an empty string. - See issue crowbartools#2801
phroggster
changed the title
|
|
This reverts commit 197fe92. Going to edit the currency add/edit controller instead. It's better for everyone.
- Checking then unchecking the offline payout option in an add or edit currency modal will assign an empty string to the data model. - The data model is typed as a Number. - This typing is exposed to external scripts. - So trim the null or empty string on addOrEditCurrency controller save to prevent a string from getting through. - See issue crowbartools#2801.
|
I have reverted my prior 2801 commit, and gone ahead and fixed it properly. Retested with every variation that I could come up with to break it, and this works better, and is better for the outside world. |
phroggster
commented
Sep 11, 2024
phroggster
commented
Sep 11, 2024
src/gui/app/directives/modals/currency/addOrEditCurrency/addOrEditCurrency.js
Show resolved
Hide resolved
CaveMobster
reviewed
Sep 27, 2024
src/gui/app/directives/modals/currency/addOrEditCurrency/addOrEditCurrency.js
Show resolved
Hide resolved
- Remove the empty string offline currency payout upon read. - Immediately re-write the file without the poorly-typed value. - Remove the empty string offline currency payout upon import, before it actually gets added or updated.
- currency-access.ts should also catch nulls, while 0's are OK. - currency-manager.ts is much more concise with a boolean nullish. - As-is currency.service.js... - ... which also had two lint warnings in it that got buffed out.
- addOrEditCurrency.js can also use a boolean nullish. - Buffed out the two arrow-parens lint warnings in there as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the Change
offlineproperty is an empty string if the checkbox is checked, then unchecked in the add or edit currency model (addOrEditCurrency.js/html).I don't want to expand the Currency type definition in currency-access.ts, but we might want to.Edit: I didn't.Applicable Issues
#2796. #2801
Testing
Minimal, but yes. Verified bugs existed, then verified that this fixed them.
Screenshots
N/A