OBPIH-7541 Create UI for new reorder report by alannadolny · Pull Request #5571 · openboxes/openboxes

alannadolny · 2025-10-27T11:20:25Z

No description provided.

codecov · 2025-10-27T11:29:03Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 8.50%. Comparing base (1bb7314) to head (d05fc15).
⚠️ Report is 160 commits behind head on develop.

Files with missing lines	Patch %	Lines
...pih/warehouse/inventory/InventoryController.groovy	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             develop   #5571      +/-   ##
============================================
- Coverage       9.12%   8.50%   -0.62%     
+ Complexity      1170    1113      -57     
============================================
  Files            701     707       +6     
  Lines          45281   45452     +171     
  Branches       10851   10886      +35     
============================================
- Hits            4131    3867     -264     
- Misses         40497   41011     +514     
+ Partials         653     574      -79

☔ 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

SebastianLib · 2025-10-27T11:29:49Z

src/js/components/cycleCountReporting/ProductsTab.jsx

    columns,
    tableData,
    loading,
-    emptyTableMessage,


Why did you remove emptyTableMessage here but not also in useProductsTab? If we really want to remove it I think we should also remove it from useProductsTab 🤔

I will ask differently - why was this file even touched? Were you just testing something for the table and forgot to bring back or?

uh, I did it by mistake 😢

SebastianLib · 2025-10-27T11:46:30Z

grails-app/i18n/messages.properties

+react.report.reorder.category.label=Category
+react.report.reorder.tags.label=Tags
+react.report.reorder.loadTable.label=Load table


From what I see, we declare the same things a few times for example "Tags" (react.productsList.filters.tags.label, react.cycleCount.filter.tags.label, and now react.report.reorder.tags.label). Maybe we should create a common key for example react.default.filters.tags. Right now, while working on my current ticket, I’m facing the same issue where I have to write the same translations multiple times for different things. What do you think @alannadolny @kchelstowski

@SebastianLib this is a larger conversation and refactor. I think the current reason for doing it this way is not to have to eventually pull translations for another feature (e.g. if we were to use the productLists.filters.tags, we'd have to inject the whole productList.* list of translations.

kchelstowski · 2025-10-28T08:20:43Z

src/js/components/Filter/FilterForm.jsx

-        onSubmit={updateFilterParams}
+        onSubmit={(values) => {
+          updateFilterParams(values);
+          onSubmit(values);


not a big deal, but wouldn't it actually be cleaner if we had the onSubmit null by default and here say:

if (onSubmit) { ... }

By just looking at this part it feels like it's always triggered (I know it's an empty function), so my suggestion would be to make an explicit if, but I leave it up to you

kchelstowski · 2025-10-28T08:21:48Z

src/js/components/cycleCountReporting/ProductsTab.jsx

    columns,
    tableData,
    loading,
-    emptyTableMessage,


I will ask differently - why was this file even touched? Were you just testing something for the table and forgot to bring back or?

kchelstowski · 2025-10-28T08:22:42Z

src/js/components/reporting/reorderReport/ReorderReport.jsx

+import 'components/cycleCount/cycleCount.scss';
+
+const ReorderReport = () => {
+  useTranslation('report');


@SebastianLib this is the reason we wouldn't want to use productList's label, because we'd have to fetch all of them here.
I agree with you in general and we have plans to somehow standardize that, but it's a larger topic.

kchelstowski · 2025-10-28T08:23:42Z

src/js/components/reporting/reorderReport/ReorderReportFilterFields.jsx

@@ -0,0 +1,86 @@
+import FilterSelectField from 'components/form-elements/FilterSelectField';
+import {
+  getExpiredStockOptions, getFilterProductOptions,


doesn't eslint shout about those two being in one line?

No, it doesn't shout, so I often forget about splitting those into separate lines

kchelstowski · 2025-10-28T08:24:45Z

src/js/components/reporting/reorderReport/ReorderReportFilters.jsx

+        showFilterVisibilityToggler={false}
+        showSearchField={false}
+        allowEmptySubmit
+        customSubmitButtonDefaultLabel="Load table"


should this be called like that if we don't have the table at the moment? I believe this should be rather Download report?

Yeah, this is how it was requested in the ticket

kchelstowski · 2025-10-28T08:26:24Z

src/js/consts/filterOptions.js

@@ -0,0 +1,102 @@
+import translate from 'utils/Translate';
+
+export const OPTION_ID = {


I think for clarity it's better to separate the enums here and have ExpirationFilter and InventoryLevelStatus like on backend.

kchelstowski · 2025-10-28T08:30:13Z

src/js/consts/filterOptions.js

@@ -0,0 +1,102 @@
+import translate from 'utils/Translate';


actually it's the first time I see such import. What method is triggered then? Isn't it actually a JSX Translate element that is returned? I'm wondering why we use useTranslate in a few places and in few places we import translate like that. I know it's a simple JS file so we can't use useTranslate, but if this works, what you have just done, why would we even need the useTranslate hook 🤔
Not something to fix, just curiosity if you already know the answer

So the translation is the TranslateWrapper function (or call it a component) that returns . It gets translated because it is in the translation context. The translate returns HTML, and it is successfully mapped to options in the select. So here is the difference between that and the useTranslate, one of them returns HTML, while the second returns a plain, translated text.

kchelstowski · 2025-10-28T08:31:43Z

src/js/hooks/reporting/useReorderReportFilters.js

+      tags: selectedTags,
+    } = queryProps;
+
+    // If there are no values for catalogs, tags, glAccounts, categories or product family


update the comment

kchelstowski · 2025-10-28T08:34:45Z

src/js/hooks/reporting/useReorderReportFilters.js

+      && !selectedFilterProducts
+      && !expiredStock
+      && !additionalInventoryLocations) {
+      setDefaultFilterValues(defaultValues);


shouldn't we return at this point?

kchelstowski · 2025-10-28T08:36:52Z

src/js/utils/option-utils.jsx

  return response.data.data;
 };

+export const fetchLocations = async ({ activityCodes }) => {


how did it end up? Was it finally be supposed to be a debounced fetch or not?

No, it doesn't need to be debounced

# Conflicts: # grails-app/i18n/messages.properties

alannadolny requested review from SebastianLib, awalkowiak and kchelstowski October 27, 2025 11:20

alannadolny self-assigned this Oct 27, 2025

SebastianLib reviewed Oct 27, 2025

View reviewed changes

kchelstowski reviewed Oct 28, 2025

View reviewed changes

alannadolny added 6 commits October 28, 2025 11:24

OBPIH-7541 Add new report filters

44e1c70

OBPIH-7541 Add new route

f88f25c

OBPIH-7541 Add new translations

7361d6b

OBPIH-7541 Add fetching locations

b1ba40c

OBPIH-7541 Adjust code to work with backend data

a5fa12a

OBPIH-7541 Fix assigning default option

9a90665

alannadolny force-pushed the ft/OBPIH-7541 branch from 173a778 to 87906fb Compare October 28, 2025 10:33

OBPIH-7541 Fixes after review

d05fc15

alannadolny force-pushed the ft/OBPIH-7541 branch from 87906fb to d05fc15 Compare October 28, 2025 10:35

SebastianLib approved these changes Oct 28, 2025

View reviewed changes

kchelstowski approved these changes Oct 28, 2025

View reviewed changes

alannadolny merged commit 29552c9 into develop Oct 29, 2025
7 checks passed

alannadolny deleted the ft/OBPIH-7541 branch October 29, 2025 09:47

SebastianLib pushed a commit that referenced this pull request Oct 30, 2025

OBPIH-7541 Create UI for new reorder report (#5571)

dd49773

# Conflicts: # grails-app/i18n/messages.properties

		@@ -0,0 +1,102 @@
		import translate from 'utils/Translate';

		export const OPTION_ID = {

Uh oh!

Conversation

alannadolny commented Oct 27, 2025

Uh oh!

codecov bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianLib Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Oct 27, 2025 •

edited

Loading

SebastianLib Oct 27, 2025 •

edited

Loading