[OSDEV-1365] SLC: Integrate collecting contribution data page#470
Conversation
…ies data, mocked sectors
Change the release date for version 1.27.0 Co-authored-by: Inessa Druzhkova <inessadruzkova@inessas-mbp.home>
…_date" is absent in object (#468) [OSDEV-1533](https://opensupplyhub.atlassian.net/browse/OSDEV-1533) - The presentation of the `Moderation Decision Date` in the `Moderation Queue` table has been corrected. If the "status_change_date" is missing in the object, it now displays as "N/A". <img width="400" alt="Screenshot 2024-12-20 at 17 10 07" src="https://github.com/user-attachments/assets/43202570-90bb-421d-aec9-2782da115081" /> [OSDEV-1533]: https://opensupplyhub.atlassian.net/browse/OSDEV-1533?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Inessa Druzhkova <inessadruzkova@inessas-mbp.home>
[OSDEV-1532](https://opensupplyhub.atlassian.net/browse/OSDEV-1532) - Fixed the date range picker on the `Moderation Queue` page. A Data Moderator can change the Before date even if an Error message is displayed. [OSDEV-1532]: https://opensupplyhub.atlassian.net/browse/OSDEV-1532?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Inessa Druzhkova <inessadruzkova@inessas-mbp.home>
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
doc/release/RELEASE-NOTES.md (4)
35-47: Improve formatting consistency in the release notes.The bullet points under OSDEV-1365 and OSDEV-1370 use inconsistent indentation and bullet point styles. Consider standardizing the format:
* [OSDEV-1365](https://opensupplyhub.atlassian.net/browse/OSDEV-1365) - SLC: Integrate collecting contribution data page. * [OSDEV-1370](https://opensupplyhub.atlassian.net/browse/OSDEV-1370) - SLC: Connect Backend API submission with "Thank for Submitting" screen: - - Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. - - Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page. + * Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. + * Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
37-37: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
37-37: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
38-38: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-40: Clarify route description for production location info page.The description of the production location info page routes could be clearer:
- - Production location info page is now rendered using two routes: /contribute/production-location/info/ and /contribute/production-location/{os_id}/info/. First route for creating new production location, second is for updating existing one. + - Production location info page is accessible via two routes: + * `/contribute/production-location/info/` - For creating new production locations + * `/contribute/production-location/{os_id}/info/` - For updating existing production locations🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
39-39: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-43: Improve clarity of moderation ID path parameter description.The description of the moderation ID path parameter attachment could be clearer:
- - Integrated "Thank for Submitting" modal dialog. When popup is appeared, path parameter `{moderation-id}` will be attached to `/contribute/production-location/{os_id}/info/` or `/contribute/production-location/info/`. + - Integrated "Thank for Submitting" modal dialog. Upon submission, the moderation ID is appended as a path parameter to either: + * `/contribute/production-location/{os_id}/info/{moderation-id}` for existing locations + * `/contribute/production-location/info/{moderation-id}` for new locations🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-47: Enhance description of search parameters storage.The description of the search parameters storage could be more detailed:
- - Refactored routing between search results page and production location info page. Search parameters are now stored in the Redux state, so the 'Go Back' button in production location info page will lead to the previous search. + - Refactored routing between search results page and production location info page: + * Search parameters are now persisted in Redux state + * The 'Go Back' button on the production location info page preserves the previous search context + * Enables seamless navigation between search results and location details🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
37-37: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-fe-cov
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
doc/release/RELEASE-NOTES.md (2)
35-47: Consider improving the documentation structure and clarity.The new features section could benefit from better organization and clarity:
- The bullet points should follow consistent formatting and indentation
- The implementation details could be better grouped by feature area
- Some technical details could use more context for better understanding
Consider restructuring like this:
* [OSDEV-1365] SLC: Integrate collecting contribution data page: -* [OSDEV-1370] - SLC: Connect Backend API submission with "Thank for Submitting" screen - - Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. - - Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page. + - API Integration: + * POST `/v1/production-locations/` for creating new locations + * PATCH `/v1/production-locations/` for updating existing locations + - Routing Implementation: + * `/contribute/production-location/info/` for new locations + * `/contribute/production-location/{os_id}/info/` for updates + - Error Handling: + * Error popups for API responses + * Error handling for moderation events + - User Experience: + * "Thank for Submitting" modal with mobile/desktop layouts + * Temporary storage of moderation events + * Improved navigation between search and info pages🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
37-37: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
37-37: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
38-38: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
35-47: Fix markdown formatting inconsistencies.The markdown formatting in this section has inconsistencies:
- List items use dashes (-) instead of asterisks (*)
- Indentation is 4 spaces instead of the expected 2 spaces
Apply these formatting changes:
- - Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. - - Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page. + * Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. + * Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
37-37: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
37-37: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
38-38: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/Routes.jsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/Routes.jsx
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
37-37: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-fe-code-quality
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
36-48: Consider improving the formatting consistency of bullet points.The bullet points in this section use inconsistent indentation and dash style. Consider standardizing to:
- * [OSDEV-1365](https://opensupplyhub.atlassian.net/browse/OSDEV-1365) - SLC: Integrate collecting contribution data page. - * [OSDEV-1370](https://opensupplyhub.atlassian.net/browse/OSDEV-1370) - SLC: Connect Backend API submission with "Thank for Submitting" screen - - Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. - - Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page. + * [OSDEV-1365](https://opensupplyhub.atlassian.net/browse/OSDEV-1365) - SLC: Integrate collecting contribution data page. + * [OSDEV-1370](https://opensupplyhub.atlassian.net/browse/OSDEV-1370) - SLC: Connect Backend API submission with "Thank for Submitting" screen + * Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. + * Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page.This improves readability and follows markdown best practices.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
38-38: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
48-48: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
48-48: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
38-38: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
48-48: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
src/react/src/components/Contribute/ProductionLocationDialog.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/Contribute/ProductionLocationDialog.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/Contribute/ProductionLocationDialog.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/Contribute/ProductionLocationDialogCloseButton.jsx
Show resolved
Hide resolved
src/react/src/components/Contribute/SearchByNameAndAddressNotFoundResult.jsx
Outdated
Show resolved
Hide resolved
src/react/src/components/Contribute/SearchByOsIdSuccessResult.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
39-51: Improve markdown formatting consistency.The list items use dashes (-) and 4-space indentation, while the markdown style guide recommends:
- Using asterisks (*) for unordered lists
- Using 2-space indentation for list items
Apply this formatting:
- * [OSDEV-1365](https://opensupplyhub.atlassian.net/browse/OSDEV-1365) - SLC: Integrate collecting contribution data page. - - Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. - - Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page. + * [OSDEV-1365](https://opensupplyhub.atlassian.net/browse/OSDEV-1365) - SLC: Integrate collecting contribution data page. + * Integrated `POST /v1/production-locations/` in `/contribute/production-location/info/` page. + * Integrated `PATCH /v1/production-locations/` in `/contribute/production-location/{os_id}/info/` page.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
48-48: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
48-48: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
49-49: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
49-49: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
50-50: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
50-50: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
51-51: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
51-51: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/util/constants.jsx(2 hunks)src/react/src/util/styles.js(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/util/constants.jsx
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
41-41: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
48-48: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
49-49: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
49-49: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
50-50: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
50-50: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
51-51: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
51-51: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
🔇 Additional comments (4)
doc/release/RELEASE-NOTES.md (1)
39-51: Well-documented integration changes!The release notes comprehensively document the integration of the collecting contribution data page, including:
- API endpoint integrations
- Routing configurations
- Error handling implementations
- UI/UX improvements
- Data serialization and validation
- Search functionality enhancements
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
41-41: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
41-41: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
42-42: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
42-42: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
43-43: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
43-43: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
44-44: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
44-44: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
45-45: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
45-45: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
46-46: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
46-46: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
47-47: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
47-47: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
48-48: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
48-48: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
49-49: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
49-49: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
50-50: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
50-50: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
51-51: Unordered list style
Expected: asterisk; Actual: dash(MD004, ul-style)
51-51: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
src/react/src/util/styles.js (3)
538-554: Well-structured close button styles implementation!The styles are properly organized with separate mobile and desktop variants, using theme values consistently and following Material-UI best practices.
583-587: Good layout improvements for text elements!The addition of
minHeightand text alignment properties helps maintain consistent spacing and layout.
Line range hint
617-658: Excellent status badge and button style improvements!The changes enhance the UI by:
- Adding distinct visual styles for each status state (pending, rejected, approved)
- Improving button styling with proper text transform and padding
- Handling claim button disabled state appropriately
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)
97-104: Consider memoizing the mobile state calculation.The mobile state calculation in useEffect could benefit from memoization to prevent unnecessary re-renders.
- useEffect(() => { - if (getIsMobile(innerWidth)) { - setIsMobile(true); - } else { - setIsMobile(false); - } - }, [innerWidth]); + const isMobileValue = useMemo(() => getIsMobile(innerWidth), [innerWidth]); + useEffect(() => { + setIsMobile(isMobileValue); + }, [isMobileValue]);src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
277-291: Consider using a custom hook for moderation event handling.The moderation event logic could be extracted into a custom hook for better reusability and separation of concerns.
+const useModerationEvent = (pendingModerationEventData, history, location) => { + useEffect(() => { + if (pendingModerationEventData?.cleaned_data) { + toast('Your contribution has been added successfully!'); + if (pendingModerationEventData?.moderation_id) { + localStorage.setItem( + pendingModerationEventData.moderation_id, + JSON.stringify(pendingModerationEventData?.cleaned_data), + ); + history.push({ + pathname: `${location.pathname}${pendingModerationEventData.moderation_id}`, + search: '', + }); + } + } + }, [pendingModerationEventData]); +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/react/src/actions/contributeProductionLocation.js(3 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(2 hunks)src/react/src/components/Contribute/ProductionLocationDialogCloseButton.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationInfo.jsx(7 hunks)src/react/src/components/Contribute/SearchByNameAndAddressNotFoundResult.jsx(2 hunks)src/react/src/components/Contribute/SearchByOsIdSuccessResult.jsx(3 hunks)src/react/src/util/util.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/react/src/components/Contribute/SearchByOsIdSuccessResult.jsx
- src/react/src/components/Contribute/SearchByNameAndAddressNotFoundResult.jsx
- src/react/src/components/Contribute/ProductionLocationDialogCloseButton.jsx
🧰 Additional context used
📓 Learnings (2)
src/react/src/util/util.js (1)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/util/util.js:1441-1472
Timestamp: 2025-01-29T09:00:01.638Z
Learning: The data passed to parseContribData() function in src/react/src/util/util.js is pre-parsed and validated before reaching the function, so additional type checking is not required.
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
🪛 Biome (1.9.4)
src/react/src/components/Contribute/ProductionLocationInfo.jsx
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-contricleaner-cov
🔇 Additional comments (12)
src/react/src/actions/contributeProductionLocation.js (4)
36-62: LGTM! Well-structured action creators.The action creators follow Redux best practices with clear naming conventions and separation of concerns.
64-84: LGTM! Robust error handling in createProductionLocation.The function properly handles API responses and errors, using
logErrorAndDispatchFailurefor consistent error handling.
88-110: LGTM! Consistent implementation in updateProductionLocation.The update function follows the same pattern as create, ensuring consistency in the codebase.
142-161: LGTM! Improved URL handling.The function now uses
makeProductionLocationURLfor better URL construction consistency.src/react/src/components/Contribute/ProductionLocationDialog.jsx (3)
41-59: LGTM! Improved claimButton implementation.The claimButton function now uses object destructuring for props and includes visual feedback for disabled state.
61-84: LGTM! Well-structured status handling.The
getStatusBadgeClassandgetTooltipTextfunctions provide clear and maintainable status management.
290-313: Extract claim button condition into a variable.The complex condition for claim button rendering should be extracted into a descriptive variable.
+ const isClaimable = + claimStatus === PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.UNCLAIMED && + moderationStatus !== MODERATION_STATUSES_ENUM.PENDING; + - {claimStatus === - PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.UNCLAIMED && - moderationStatus !== - MODERATION_STATUSES_ENUM.PENDING ? ( + {isClaimable ? (src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
154-169: LGTM! Robust submit method handling.The switch statement properly handles different submit methods with a default case for error handling.
739-741: Extract onClick handler into a named function.The inline onClick handler should be extracted into a named function for better readability and maintainability.
+ const handleSubmit = () => { + handleProductionLocation(inputData, osID); + }; + <Button color="secondary" variant="contained" - onClick={() => { - handleProductionLocation(inputData, osID); - }} + onClick={handleSubmit} className={classes.submitButtonStyles} >src/react/src/util/util.js (3)
262-265: LGTM! Clean URL construction.The
makeProductionLocationURLfunction follows the codebase's URL construction pattern and handles optional parameters well.
1108-1119: LGTM! Robust range field conversion.The
convertRangeFieldfunction properly handles all edge cases for range objects.
1441-1478: LGTM! Clean data transformation.The
parseContribDatafunction effectively transforms form data into the required API format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (4)
74-84: Simplify getTooltipText function using early returns.The function can be more readable with early returns instead of nested if statements.
const getTooltipText = (claimStatus, moderationStatus) => { - if (moderationStatus === MODERATION_STATUSES_ENUM.PENDING) { - return "You'll be able to claim the location after the moderation is done."; - } - - if (claimStatus === PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.CLAIMED) { - return 'Production location has been claimed already.'; - } - - return ''; + if (moderationStatus === MODERATION_STATUSES_ENUM.PENDING) + return "You'll be able to claim the location after the moderation is done."; + + if (claimStatus === PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.CLAIMED) + return 'Production location has been claimed already.'; + + return ''; };
97-103: Extract mobile detection logic into a custom hook.Consider moving the mobile detection logic into a custom hook for better reusability across components.
+const useIsMobile = (innerWidth) => { + const [isMobile, setIsMobile] = useState(false); + const getIsMobileMemoized = useMemo(() => getIsMobile(innerWidth), [innerWidth]); + + useEffect(() => { + setIsMobile(getIsMobileMemoized); + }, [getIsMobileMemoized]); + + return isMobile; +}; const ProductionLocationDialog = ({...}) => { const history = useHistory(); - const [isMobile, setIsMobile] = useState(false); - const getIsMobileMemoized = useMemo(() => getIsMobile(innerWidth), [innerWidth]); - useEffect(() => { - setIsMobile(getIsMobileMemoized); - }, [getIsMobileMemoized]); + const isMobile = useIsMobile(innerWidth);
116-127: Simplify and improve readability of filtering logic.The filtering logic can be made more readable by extracting the value validation into a separate function.
+const isValidValue = (value) => { + if (isArray(value)) return value.length > 0; + if (value && typeof value === 'object') return !isEmpty(value); + return value !== null && value !== undefined && value !== ''; +}; const filteredAdditionalFields = pickBy( additionalInformationFields, - value => { - if (isArray(value)) { - return value.length > 0; - } - if (value && typeof value === 'object') { - return !isEmpty(value); - } - return value !== null && value !== undefined && value !== ''; - }, + isValidValue );
269-288: Extract routing handlers into named functions.The inline routing handlers should be extracted into named functions for better readability and reusability.
+const handleSearchOSHub = (history) => () => { + history.push(mainRoute); +}; +const handleSubmitAnotherLocation = (history) => () => { + history.push(searchByNameAndAddressResultRoute); +}; <Button variant="contained" color="default" - onClick={() => history.push(mainRoute)} + onClick={handleSearchOSHub(history)} className={classes.button} > Search OS Hub </Button> <Button variant="contained" color="secondary" - onClick={() => { - history.push(searchByNameAndAddressResultRoute); - }} + onClick={handleSubmitAnotherLocation(history)} className={classes.button} > Submit another Location </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/Contribute/ProductionLocationDialog.jsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
🔇 Additional comments (2)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (2)
321-335: LGTM! Props are well defined.The props are properly typed with PropTypes and default values are provided for optional props.
337-339: LGTM! Component export follows Material-UI best practices.The component is properly wrapped with withTheme and withStyles HOCs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (4)
74-84: Consider simplifying the tooltip text logic.The
getTooltipTextfunction could be simplified using an object lookup or early return pattern.const getTooltipText = (claimStatus, moderationStatus) => { - if (moderationStatus === MODERATION_STATUSES_ENUM.PENDING) { - return "You'll be able to claim the location after the moderation is done."; - } - - if (claimStatus === PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.CLAIMED) { - return 'Production location has been claimed already.'; - } - - return ''; + const tooltipTexts = { + [MODERATION_STATUSES_ENUM.PENDING]: "You'll be able to claim the location after the moderation is done.", + [`${PRODUCTION_LOCATION_CLAIM_STATUSES_ENUM.CLAIMED}`]: 'Production location has been claimed already.', + }; + return tooltipTexts[moderationStatus] || tooltipTexts[claimStatus] || ''; };
97-103: Consider using a media query hook for better mobile detection.Instead of manually tracking window width, consider using a custom hook or library like
@material-ui/core/useMediaQueryfor more reliable mobile detection.- const [isMobile, setIsMobile] = useState(false); - const getIsMobileMemoized = useMemo(() => getIsMobile(innerWidth), [ - innerWidth, - ]); - useEffect(() => { - setIsMobile(getIsMobileMemoized); - }, [getIsMobileMemoized]); + const isMobile = useMediaQuery(theme.breakpoints.down('sm'));
116-127: Simplify the field filtering logic.The current field filtering logic is complex and could be simplified for better maintainability.
- const filteredAdditionalFields = pickBy( - additionalInformationFields, - value => { - if (isArray(value)) { - return value.length > 0; - } - if (value && typeof value === 'object') { - return !isEmpty(value); - } - return value !== null && value !== undefined && value !== ''; - }, - ); + const isNotEmpty = value => { + if (!value) return false; + if (Array.isArray(value)) return value.length > 0; + if (typeof value === 'object') return !isEmpty(value); + return value !== ''; + }; + const filteredAdditionalFields = pickBy(additionalInformationFields, isNotEmpty);
241-254: Improve the Chip component's onDelete handling.The current implementation adds an empty onDelete handler conditionally. Consider using the
onClickprop for the info icon instead.<Chip label={statusLabel} - {...(moderationStatus === - MODERATION_STATUSES_ENUM.PENDING && { - onDelete: () => {}, - })} className={`${ classes.osIdStatusBadge } ${getStatusBadgeClass( classes, moderationStatus, )}`} - deleteIcon={deleteIcon} + icon={moderationStatus === MODERATION_STATUSES_ENUM.PENDING ? infoIcon(classes) : null} + onClick={moderationStatus === MODERATION_STATUSES_ENUM.PENDING ? () => {} : null} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/components/Contribute/ProductionLocationDialog.jsx(2 hunks)src/react/src/components/FacilityListItems.jsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (4)
src/react/src/components/FacilityListItems.jsx (2)
230-231: LGTM! Good refactoring of navigation logic.Extracting the navigation logic into a named function improves code maintainability and follows React best practices.
317-317: LGTM! Consistent usage of the extracted navigation handler.The usage of
handleGoToMainPagemaintains functionality while improving code organization.src/react/src/components/Contribute/ProductionLocationDialog.jsx (2)
Line range hint
1-35: LGTM! Well-organized imports.The imports are properly structured with individual lodash utilities for better tree-shaking.
314-331: LGTM! Well-defined PropTypes and exports.The PropTypes are properly defined with required flags, and default props are provided where needed. The component is correctly wrapped with theme and styles HOCs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (3)
97-103: Optimize mobile check implementation.The current implementation could be more efficient by moving the
getIsMobilefunction inside the component and memoizing it directly.Consider this optimization:
- const getIsMobileMemoized = useMemo(() => getIsMobile(innerWidth), [ - innerWidth, - ]); - useEffect(() => { - setIsMobile(getIsMobileMemoized); - }, [getIsMobileMemoized]); + useEffect(() => { + setIsMobile(innerWidth <= 960); + }, [innerWidth]);
116-120: Simplify value validation logic.The
isValidValuefunction could be more concise while maintaining readability.Consider this simplification:
- const isValidValue = value => { - if (isArray(value)) return value.length > 0; - if (value && typeof value === 'object') return !isEmpty(value); - return value !== null && value !== undefined && value !== ''; - }; + const isValidValue = value => + (isArray(value) && value.length > 0) || + (value && typeof value === 'object' && !isEmpty(value)) || + (value !== null && value !== undefined && value !== '');
147-307: Consider extracting dialog sections into components.The dialog content is quite lengthy and could benefit from being broken down into smaller, reusable components.
Consider extracting these sections:
- Dialog header (title and close button)
- Facility information section
- OS ID section with status badge
- Action buttons section
Example for the OS ID section:
const OSIDSection = ({ osID, statusLabel, moderationStatus, classes }) => ( <Grid container className={classes.primaryText}> <Grid item> {osID && <Typography className={classes.osIDText}>{osID}</Typography>} <StatusBadge label={statusLabel} status={moderationStatus} classes={classes} /> </Grid> </Grid> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/Contribute/ProductionLocationDialog.jsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (3)
src/react/src/components/Contribute/ProductionLocationDialog.jsx (3)
Line range hint
1-35: Well-structured imports!Good practices observed:
- Individual lodash imports for better tree-shaking
- Proper type checking with PropTypes
- React hooks for state management
41-84: Helper functions are well-implemented!The functions are well-structured with:
- Clear parameter handling with defaults
- Comprehensive status mapping
- Early returns for better readability
311-328: PropTypes and exports are well-defined!Good practices observed:
- Comprehensive prop type definitions
- Default values for optional props
- Proper HOC composition
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (3)
7-7: Rename the importedtoStringfunction to avoid shadowing.The imported
toStringfunction from lodash shadows the globaltoStringproperty, which could lead to confusion. Consider renaming it to be more specific.Apply this diff:
-import { endsWith, isEmpty, isNil, omitBy, toString } from 'lodash'; +import { endsWith, isEmpty, isNil, omitBy, toString as toStringLodash } from 'lodash';🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
334-355: Consider using a cleanup function for the setTimeout.The useEffect hook that handles moderation event errors uses setTimeout but doesn't clean up the timer. This could lead to memory leaks or race conditions if the component unmounts before the timeout completes.
Apply this diff:
useEffect(() => { if ( moderationID && !singleModerationEventItemFetching && singleModerationEventItemError && !localStorage.getItem(moderationID) ) { - setTimeout(() => { + const timer = setTimeout(() => { toast(toString(singleModerationEventItemError)); }, 0); + return () => clearTimeout(timer); } }, [ singleModerationEventItemFetching, singleModerationEventItemError, moderationID, ]);
743-745: Optimize form submission handler.The form submission handler could be memoized to prevent unnecessary re-renders.
Apply this diff:
+ const handleSubmit = useCallback(() => { + handleProductionLocation(inputData, osID); + }, [handleProductionLocation, inputData, osID]); // ... in JSX ... - onClick={() => { - handleProductionLocation(inputData, osID); - }} + onClick={handleSubmit}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx(7 hunks)
🧰 Additional context used
📓 Learnings (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
🪛 Biome (1.9.4)
src/react/src/components/Contribute/ProductionLocationInfo.jsx
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
154-168: Proper error handling for unsupported submit methods.The switch statement now includes a default case that handles unsupported submit methods, making the code more robust.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
7-7: Consider renaming the importedtoStringfunction.The imported
toStringfrom lodash shadows the globaltoStringproperty, which could lead to confusion. Consider renaming it to be more specific, e.g.,toStringValue.Apply this diff:
-import { endsWith, isEmpty, isNil, omitBy, toString } from 'lodash'; +import { endsWith, isEmpty, isNil, omitBy, toString as toStringValue } from 'lodash';Then update all usages of
toStringtotoStringValue.🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
154-168: Simplify the handleProductionLocation switch statement.The switch statement can be simplified using an object literal for better maintainability and readability.
Apply this diff:
- let handleProductionLocation; - switch (submitMethod) { - case 'POST': - handleProductionLocation = handleCreateProductionLocation; - break; - case 'PATCH': - handleProductionLocation = data => - handleUpdateProductionLocation(data, osID); - break; - default: - handleProductionLocation = () => { - console.error(`Unsupported submit method: ${submitMethod}`); - }; - break; - } + const handlers = { + POST: handleCreateProductionLocation, + PATCH: data => handleUpdateProductionLocation(data, osID), + default: () => console.error(`Unsupported submit method: ${submitMethod}`), + }; + const handleProductionLocation = handlers[submitMethod] || handlers.default;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx(7 hunks)
🧰 Additional context used
📓 Learnings (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
🪛 Biome (1.9.4)
src/react/src/components/Contribute/ProductionLocationInfo.jsx
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (2)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
743-745: 🛠️ Refactor suggestionPrevent default form behavior in onClick handler.
The onClick handler for form submission should prevent the default behavior to ensure proper form handling.
Apply this diff:
- onClick={() => { + onClick={(e) => { + e.preventDefault(); handleProductionLocation(inputData, osID); }}Likely invalid or redundant comment.
868-875: Verify API endpoint implementations.The component interacts with multiple API endpoints. Let's verify their implementation and error handling.
Run the following script to check the API endpoints implementation:
✅ Verification successful
API endpoints are properly implemented with robust error handling.
The codebase demonstrates:
- Consistent error handling across all API endpoints
- Proper state management integration
- Appropriate user feedback mechanisms
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check API endpoint implementations and their error handling # Search for API endpoint definitions echo "Searching for API endpoint definitions..." rg -A 5 "createProductionLocation|updateProductionLocation|fetchSingleModerationEvent|fetchProductionLocationByOsId" # Search for error handling patterns echo "Checking error handling patterns..." rg "catch|error|try" --type jsLength of output: 70339
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (5)
7-7: Avoid shadowing the global toString property.The
toStringimport from lodash shadows the globaltoStringproperty, which could lead to confusion. Consider renaming the import to be more specific.Apply this diff:
-import { endsWith, isEmpty, isNil, omitBy, toString } from 'lodash'; +import { endsWith, isEmpty, isNil, omitBy, toString as toStringLodash } from 'lodash';🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
101-111: Consider memoizing the input data object.The
inputDataobject is recreated on every render. Consider usinguseMemoto optimize performance, especially since this object is used in the submit handler.Apply this diff:
- const inputData = { + const inputData = useMemo(() => ({ name: inputName, address: inputAddress, country: inputCountry, sector, productType, locationType, processingType, numberOfWorkers, parentCompany, - }; + }), [ + inputName, + inputAddress, + inputCountry, + sector, + productType, + locationType, + processingType, + numberOfWorkers, + parentCompany, + ]);
307-326: Consider extracting URL manipulation logic.The URL manipulation logic in this useEffect is complex and handles multiple responsibilities. Consider extracting it into a custom hook for better reusability and testing.
Create a new custom hook:
const useTrailingSlashUrl = (pathname, history) => { useEffect(() => { if (pathname && !pathname.endsWith('/')) { history.replace(`${pathname}/`); } }, [pathname, history]); };Then use it in the component:
- useEffect(() => { - // Force trailing slash in URL to prevent broken UX scenarios - if (location.pathname && !location.pathname.endsWith('/')) { - history.replace(`${location.pathname}/`); - } + useTrailingSlashUrl(location.pathname, history); + useEffect(() => { const unListen = history.listen(appLocation => { if ( appLocation.pathname === productionLocationInfoRouteCommon || appLocation.pathname === `${productionLocationInfoRouteCommon}${osID}/info/` ) { setShowProductionLocationDialog(false); } }); return () => { unListen(); }; - }, [location.pathname, history, osID]); + }, [history, osID]);
746-756: Add loading state to submit button.The submit button should show a loading state during form submission to prevent double submissions and provide better user feedback.
Apply this diff:
<Button color="secondary" variant="contained" onClick={() => { handleProductionLocation(inputData, osID); }} className={classes.submitButtonStyles} + disabled={pendingModerationEventFetching} > - {submitButtonText} + {pendingModerationEventFetching ? 'Submitting...' : submitButtonText} </Button>
832-867: Consider using reselect for memoized state selection.The
mapStateToPropsfunction performs complex object destructuring on every state change. Consider using reselect to memoize these selections and prevent unnecessary re-renders.Example implementation:
import { createSelector } from 'reselect'; const getCountriesOptions = state => state.filterOptions.countries.data; const getFacilityProcessingTypeOptions = state => state.filterOptions.facilityProcessingType.data; const getPendingModerationEvent = state => state.contributeProductionLocation.pendingModerationEvent; const getSingleProductionLocation = state => state.contributeProductionLocation.singleProductionLocation; const getSingleModerationEvent = state => state.dashboardContributionRecord.singleModerationEvent; const getInnerWidth = state => state.ui.window.innerWidth; const getSearchParameters = state => state.searchParameters; const mapStateToProps = createSelector( [ getCountriesOptions, getFacilityProcessingTypeOptions, getPendingModerationEvent, getSingleProductionLocation, getSingleModerationEvent, getInnerWidth, getSearchParameters, ], ( countriesOptions, facilityProcessingTypeOptions, pendingModerationEvent, singleProductionLocation, singleModerationEvent, innerWidth, searchParameters, ) => ({ countriesOptions, facilityProcessingTypeOptions, pendingModerationEventData: pendingModerationEvent.data, pendingModerationEventFetching: pendingModerationEvent.fetching, pendingModerationEventError: pendingModerationEvent.error, singleProductionLocationData: singleProductionLocation.data, singleModerationEventItem: singleModerationEvent.data, singleModerationEventItemFetching: singleModerationEvent.fetching, singleModerationEventItemError: singleModerationEvent.error, innerWidth, searchParameters, }) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx(7 hunks)
🧰 Additional context used
📓 Learnings (1)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:759-761
Timestamp: 2025-01-31T11:33:07.405Z
Learning: In the Open Supply Hub codebase, when parsing localStorage items with JSON.parse, the existence check is done in the outer condition using localStorage.getItem() before attempting to parse, rather than wrapping the parse operation itself in a null check.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:680-692
Timestamp: 2025-01-23T11:10:17.929Z
Learning: The ProductionLocationInfo component in Open Supply Hub implements field-level validation for name, address and country fields using Material-UI's TextField validation patterns and touch states, providing real-time feedback to users.
🪛 Biome (1.9.4)
src/react/src/components/Contribute/ProductionLocationInfo.jsx
[error] 7-7: Do not shadow the global "toString" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-fe-code-quality
|
5 participants
SLC: Integrate collecting contribution data page
Fix OSDEV-1365
POST /v1/production-locations/in/contribute/production-location/info/page.PATCH /v1/production-locations/in/contribute/production-location/{os_id}/info/page./contribute/production-location/info/and/contribute/production-location/{os_id}/info/. First route for creating new production location, second is for updating existing one.PATCH | POST /v1/production-locations/.GET /v1/moderation-events/{moderation_id}.{moderation-id}will be attached to/contribute/production-location/{os_id}/info/or/contribute/production-location/info/.