-
Notifications
You must be signed in to change notification settings - Fork 181
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
Fix #6563: Fixed In Transit Sorting in Warehouse Tab #6585
base: master
Are you sure you want to change the base?
Conversation
- Implemented a new `StringAndThenNumberSorter` comparator to handle sorting of strings with embedded numeric values. - Updated `WarehouseTab` to use `StringAndThenNumberSorter` for the `COL_STATUS` column in the parts table. - Enhanced sorting behavior to properly order entries with mixed alphanumeric content, i.e. `In Transit (6 days)` will appear before `In Transit (10 days)`, but both will appear after `Functional`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6585 +/- ##
=========================================
Coverage 11.43% 11.43%
- Complexity 6451 6452 +1
=========================================
Files 1087 1088 +1
Lines 139447 139467 +20
Branches 21560 21565 +5
=========================================
+ Hits 15944 15948 +4
- Misses 121908 121922 +14
- Partials 1595 1597 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
*/ | ||
private static final Pattern NUMBER_PATTERN = Pattern.compile("(\\d+|\\D+)"); | ||
|
||
/** |
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.
Pattern is incorrect. that will match all numbers and non-numbers.
To match all numbers it is \d+
(or for that variation \\d+
)
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.
If it's only displayed in days, the \\d+
will work. Otherwise you'll need to adjust it to be \\(((\\d+) (\\D+)\\)
to capture the number of days AND interval to compare days, weeks, months, years...
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'm not sure what I need to do here, could you provide a little more information?
For context, the table column uses a mix of both numbers and non-numbers. For example, the column might have 'damaged', 'functional', 'in transit (5 days)', 'in transit (353 days)'. The intention is that it will sort based on letters and then, when it hits a number, start sorting based on that.
Right now, the implementation visually works correctly, but if I move it to \\d+
I run into Comparison method violates its general contract!
errors, presumably because I'm left with empty strings to compare because all the non-digits get filtered out.
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.
Also, if it helps, it only appears in batches of 'days', we don't convert that to months or similar
StringAndThenNumberSorter
comparator to handle sorting of strings with embedded numeric values.WarehouseTab
to useStringAndThenNumberSorter
for theCOL_STATUS
column in the parts table.In Transit (6 days)
will appear beforeIn Transit (10 days)
, but both will appear afterFunctional
.Fix #6563
Dev Notes
RegEx is freakin' sorcery, I swear.
I stole this pattern from the internet so don't ask me how it works, I just know it does.