[OSDEV-2084] Enable Self Service Data Upload (Dromo)#692
[OSDEV-2084] Enable Self Service Data Upload (Dromo)#692mazursasha1990 merged 29 commits intomainfrom
Conversation
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 35.27%Your code coverage diff: 0.24% ▴ ✅ All code changes are covered |
📝 WalkthroughWalkthroughThis update introduces a new "Beta Self Service Upload" feature for facility list file uploads, controlled by a backend feature flag ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContributeForm
participant SelfServiceDromoUploader
participant DromoUploader
participant processDromoResults
participant FileInput
User->>ContributeForm: Loads page
ContributeForm->>SelfServiceDromoUploader: Renders if ENABLE_DROMO_UPLOADING is true
User->>SelfServiceDromoUploader: Clicks "Beta Self Service Upload"
SelfServiceDromoUploader->>DromoUploader: Opens uploader dialog
DromoUploader->>User: User uploads file and submits
DromoUploader-->>SelfServiceDromoUploader: Returns results and metadata
SelfServiceDromoUploader->>processDromoResults: Passes results, filename, fileInput, updateFileName
processDromoResults->>FileInput: Updates file input with generated CSV file
processDromoResults->>SelfServiceDromoUploader: Calls updateFileName callback
SelfServiceDromoUploader->>DromoUploader: Closes uploader dialog
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/react/src/components/ContributeForm.jsx (2)
123-123: Use optional chaining for cleaner code.The condition can be simplified using optional chaining as suggested by the static analysis tool.
Apply this diff:
- if (fileInput && fileInput.current) { + if (fileInput?.current) {
200-232: Improve UI accessibility and styling consistency.The new button layout has some areas for improvement:
- Hardcoded color: The green color
#62CC74should use the color constants- Missing accessibility: No ARIA labels or descriptions for the beta feature
- Inconsistent styling: Mixed inline styles with existing class-based approach
Apply this diff to improve accessibility and consistency:
<div style={{ display: 'flex', gap: '10px' }}> <MaterialButton onClick={selectFile} type="button" variant="outlined" color="primary" className="outlined-button" disableRipple + aria-label="Select facility list file from your computer" > Select Facility List File </MaterialButton> <MaterialButton onClick={openDromoUploader} type="button" variant="contained" style={{ - backgroundColor: '#62CC74', + backgroundColor: COLOURS.GREEN, // Add to COLOURS if not exists color: 'white', }} disableRipple + aria-label="Use beta self-service upload with data validation" > Beta Self Service Upload </MaterialButton> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/public/index.html(2 hunks)src/react/src/components/ContributeForm.jsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/react/public/index.html (1)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/__tests__/components/ProductionLocationDialog.test.js:66-105
Timestamp: 2025-01-27T07:57:17.370Z
Learning: In the Open Supply Hub project, accessibility testing is performed manually rather than through automated tests.
src/react/src/components/ContributeForm.jsx (7)
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/react/src/components/ContributeForm.jsx:175-206
Timestamp: 2024-11-20T23:08:05.475Z
Learning: In the `ContributeForm.jsx` React component, it's acceptable to include an empty `onClick={() => {}}` handler on a disabled `<Button>` to prevent console warnings.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.
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.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/components/Contribute/ProductionLocationInfo.jsx:229-243
Timestamp: 2025-01-23T10:19:59.642Z
Learning: In React useEffect hooks that perform navigation, adding router-related dependencies (history, location) to the dependency array can cause unwanted re-renders and side effects. For navigation effects triggered by data changes, include only the data dependencies.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#437
File: src/react/src/components/Filters/StyledSelect.jsx:31-39
Timestamp: 2024-12-05T10:43:59.922Z
Learning: In the `StyledSelect` component (`src/react/src/components/Filters/StyledSelect.jsx`), the `label` prop is optional. The `InputLabel` should be conditionally rendered only when a label is provided to prevent unnecessary DOM elements, extra padding, and browser console warnings.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/Contribute/ProductionLocationDialog.jsx:33-181
Timestamp: 2024-12-10T07:09:35.641Z
Learning: In the `ProductionLocationDialog.jsx` component, hardcoded facility data is acceptable for now as per the project's requirements.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/react/src/components/Button.jsx:43-43
Timestamp: 2024-11-22T12:22:59.701Z
Learning: In the Open Supply Hub codebase, the Button component in `src/react/src/components/Button.jsx` is designed to have an optional `onClick` prop to accommodate different cases.
🧬 Code Graph Analysis (1)
src/react/src/components/ContributeForm.jsx (2)
src/react/src/util/apiRequest.js (1)
headers(12-15)src/react/src/util/hooks.js (1)
fileInput(353-353)
🪛 Biome (1.9.4)
src/react/src/components/ContributeForm.jsx
[error] 123-123: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-dd-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-django-code-quality
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
🔇 Additional comments (1)
src/react/src/components/ContributeForm.jsx (1)
1-1: LGTM!The useEffect import is correctly added to support the Dromo uploader initialization.
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 55.73%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.75%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
…romo_uploading_switch migration
There was a problem hiding this comment.
Pull Request Overview
The PR enables a new self-service data upload flow using the Dromo uploader behind a feature flag.
- Adds a utility (
processDromoResults) to convert Dromo JSON results into a CSV file and inject it into the existing file input. - Introduces
SelfServiceDromoUploadercomponent and integrates it intoContributeFormunder theENABLE_DROMO_UPLOADINGflag. - Updates constants, propTypes, adds a migration for the feature flag, dependency, and tests for the new functionality.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/react/src/util/util.js | Added processDromoResults to serialize results to CSV and update input |
| src/react/src/util/propTypes.js | Imported and added ENABLE_DROMO_UPLOADING to feature flags |
| src/react/src/util/constants.jsx | Defined ENABLE_DROMO_UPLOADING constant |
| src/react/src/components/SelfServiceDromoUploader.jsx | New component for launching and handling Dromo uploader |
| src/react/src/components/ContributeForm.jsx | Integrated uploader button and hint into the main form |
| src/react/src/tests/utils.tests.js | Unit tests for processDromoResults |
| src/react/src/tests/components/SelfServiceDromoUploader.test.jsx | Component tests for SelfServiceDromoUploader |
| src/react/package.json | Added dromo-uploader-react dependency |
| src/django/api/migrations/0176_introduce_enable_dromo_uploading_switch .py | Migration to add the Dromo uploader feature flag |
src/django/api/migrations/0176_introduce_enable_dromo_uploading_switch .py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/react/src/util/util.js (1)
1733-1735: CSV generation vulnerability already identified.The naive CSV generation approach can produce malformed output when data contains commas, quotes, or newlines. This issue was already flagged in previous reviews.
🧹 Nitpick comments (3)
src/react/src/__tests__/components/SelfServiceDromoUploader.test.jsx (1)
1-90: Consider adding edge case tests for enhanced coverage.The current test suite covers the main functionality well. Consider adding tests for:
- Component behavior when
fileInputorupdateFileNameprops are null/undefined- Error handling scenarios (if the component handles any)
- Multiple rapid clicks on the upload button
doc/release/RELEASE-NOTES.md (1)
34-36: Indentation violates Markdown-lint (MD007)Sub-bullets should be indented two spaces under the parent list, not four. This is already flagged by markdownlint-cli2.
- * `migrate` - * `reindex_database` + * `migrate` + * `reindex_database`src/react/src/__tests__/utils.tests.js (1)
2714-2727: Consider verifying CSV content creationThe test validates file creation and properties but doesn't verify that the CSV content was generated correctly from the input data.
Consider adding content verification:
it('should process results and update file input', () => { const results = [ { name: 'Alice', age: 30 }, { name: 'Bob', age: 25 }, ]; processDromoResults(results, 'file.xlsx', fileInput, updateFileName); expect(fileInput.current.files).toHaveLength(1); const file = fileInput.current.files[0]; expect(file.name).toBe('file.csv'); expect(file.options.type).toBe('text/csv'); + + // Verify CSV content structure + expect(file.content).toContain('name,age'); + expect(file.content).toContain('Alice,30'); + expect(file.content).toContain('Bob,25'); expect(updateFileName).toHaveBeenCalledWith(fileInput); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
doc/release/RELEASE-NOTES.md(1 hunks)src/django/api/migrations/0176_introduce_enable_dromo_uploading_switch.py(1 hunks)src/react/package.json(1 hunks)src/react/src/__tests__/components/SelfServiceDromoUploader.test.jsx(1 hunks)src/react/src/__tests__/utils.tests.js(2 hunks)src/react/src/components/ContributeForm.jsx(4 hunks)src/react/src/components/SelfServiceDromoUploader.jsx(1 hunks)src/react/src/util/constants.jsx(1 hunks)src/react/src/util/propTypes.js(2 hunks)src/react/src/util/util.js(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/react/package.json
- src/react/src/util/constants.jsx
- src/django/api/migrations/0176_introduce_enable_dromo_uploading_switch.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/ContributeForm.jsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/django/api/management/commands/enable_switches.py:15-15
Timestamp: 2024-11-20T23:12:50.325Z
Learning: In `src/django/api/management/commands/enable_switches.py`, setting `'disable_list_uploading'` to `'off'` is intentional to enable list uploading outside of the release process.
src/react/src/util/propTypes.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/__tests__/components/SelfServiceDromoUploader.test.jsx (4)
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#483
File: src/react/src/__tests__/components/SearchByNameAndAddressSuccessResult.test.js:0-0
Timestamp: 2025-01-17T16:12:18.285Z
Learning: In the SearchByNameAndAddressSuccessResult component's tests, attempting to test internal navigation logic through mocking useHistory is not feasible, and button presence/click events should be covered in the main rendering test case.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/DialogTooltip.jsx:0-0
Timestamp: 2024-12-03T10:29:02.409Z
Learning: In the `DialogTooltip` component (`src/react/src/components/DialogTooltip.jsx`), there are existing tooltip tests that cover common cases, and further detailed tests are not necessary.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#483
File: src/react/src/__tests__/components/SearchByNameAndAddressSuccessResult.test.js:110-118
Timestamp: 2025-01-17T15:04:49.076Z
Learning: In the SearchByNameAndAddressSuccessResult component, the Select functionality is implemented internally within the component rather than being passed as props, making it unsuitable for direct function mocking in tests.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/__tests__/components/ProductionLocationDialog.test.js:0-0
Timestamp: 2025-01-27T07:20:43.334Z
Learning: In the Open Supply Hub project, buttons wrapped by the `DialogTooltip` component control their disabled state through CSS pointer-events rather than the HTML disabled attribute, requiring tests to check `getComputedStyle().pointerEvents` instead of using `toBeDisabled()`.
src/react/src/components/SelfServiceDromoUploader.jsx (4)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/react/src/components/Button.jsx:43-43
Timestamp: 2024-11-22T12:22:59.701Z
Learning: In the Open Supply Hub codebase, the Button component in `src/react/src/components/Button.jsx` is designed to have an optional `onClick` prop to accommodate different cases.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#437
File: src/react/src/components/Filters/StyledSelect.jsx:31-39
Timestamp: 2024-12-05T10:43:59.922Z
Learning: In the `StyledSelect` component (`src/react/src/components/Filters/StyledSelect.jsx`), the `label` prop is optional. The `InputLabel` should be conditionally rendered only when a label is provided to prevent unnecessary DOM elements, extra padding, and browser console warnings.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/react/src/components/ContributeForm.jsx:175-206
Timestamp: 2024-11-20T23:08:05.475Z
Learning: In the `ContributeForm.jsx` React component, it's acceptable to include an empty `onClick={() => {}}` handler on a disabled `<Button>` to prevent console warnings.
src/react/src/__tests__/utils.tests.js (4)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/DialogTooltip.jsx:0-0
Timestamp: 2024-12-03T10:29:02.409Z
Learning: In the `DialogTooltip` component (`src/react/src/components/DialogTooltip.jsx`), there are existing tooltip tests that cover common cases, and further detailed tests are not necessary.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#483
File: src/react/src/__tests__/components/SearchByNameAndAddressSuccessResult.test.js:0-0
Timestamp: 2025-01-17T16:12:18.285Z
Learning: In the SearchByNameAndAddressSuccessResult component's tests, attempting to test internal navigation logic through mocking useHistory is not feasible, and button presence/click events should be covered in the main rendering test case.
Learnt from: roman-stolar
PR: opensupplyhub/open-supply-hub#448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#511
File: src/react/src/util/util.js:1436-1447
Timestamp: 2025-02-12T11:45:01.562Z
Learning: The `generateRangeField` utility function in `src/react/src/util/util.js` is meant for data transformation only. Input validation is handled separately on the frontend for specific input fields.
src/react/src/util/util.js (2)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#511
File: src/react/src/util/util.js:1436-1447
Timestamp: 2025-02-12T11:45:01.562Z
Learning: The `generateRangeField` utility function in `src/react/src/util/util.js` is meant for data transformation only. Input validation is handled separately on the frontend for specific input fields.
Learnt from: roman-stolar
PR: opensupplyhub/open-supply-hub#448
File: src/react/src/util/util.js:1317-1317
Timestamp: 2024-12-10T15:07:54.819Z
Learning: Changes to date handling functions like `formatDate` in `src/react/src/util/util.js` should be made carefully, considering that different parts of the codebase may intentionally use local time instead of UTC, and broad changes to `moment()` usage may not be appropriate without context.
doc/release/RELEASE-NOTES.md (7)
undefined
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #641
File: doc/release/RELEASE-NOTES.md:6-35
Timestamp: 2025-06-02T13:24:57.659Z
Learning: The Open Supply Hub team keeps placeholders in release notes until code freeze, then fills in the actual content once all changes are finalized for the release.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:37-37
Timestamp: 2024-11-25T13:28:23.090Z
Learning: When modifying unreleased API endpoints, such as refactoring the search_after parameter into search_after_value and search_after_id in the OpenSearch implementation, it's acceptable to leave the "What's New" section empty since the changes haven't been released to end users.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:47-54
Timestamp: 2024-11-28T06:36:47.122Z
Learning: Endpoints that have not been enabled to end users do not require migration documentation or old vs new format examples in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:38-38
Timestamp: 2024-11-26T04:59:12.296Z
Learning: For endpoints that haven't been released to end users, it's acceptable to document API changes under the 'Bugfix' section in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:47-54
Timestamp: 2024-11-28T06:36:47.122Z
Learning: API changes and migration details are documented in a separate repository with API specifications, rather than in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: vladsha-dev
PR: #490
File: deployment/environments/terraform-preprod.tfvars:17-18
Timestamp: 2025-01-29T13:36:47.477Z
Learning: In the Open Supply Hub project, the pre-prod environment is always created from scratch, so PostgreSQL version upgrade flags (rds_allow_major_version_upgrade and rds_apply_immediately) are not needed as there's no upgrade process involved. These flags should only be set for environments that are being upgraded from an existing PostgreSQL 13 instance (e.g., Staging, Production, Dev, Test).
</retrieved_learning>
<retrieved_learning>
Learnt from: Innavin369
PR: #415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
</retrieved_learning>
🧬 Code Graph Analysis (3)
src/react/src/util/propTypes.js (1)
src/react/src/util/constants.jsx (2)
ENABLE_DROMO_UPLOADING(566-566)ENABLE_DROMO_UPLOADING(566-566)
src/react/src/__tests__/components/SelfServiceDromoUploader.test.jsx (1)
src/react/src/components/SelfServiceDromoUploader.jsx (1)
SelfServiceDromoUploader(12-46)
src/react/src/util/util.js (2)
src/react/src/__tests__/utils.tests.js (2)
fileInput(2655-2655)updateFileName(2656-2656)src/react/src/util/hooks.js (1)
fileInput(353-353)
🪛 Gitleaks (8.27.2)
src/react/src/components/SelfServiceDromoUploader.jsx
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-flake8-linter
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-fe-code-quality
🔇 Additional comments (18)
src/react/src/util/propTypes.js (2)
34-34: LGTM: Correct import addition for new feature flag.The import of
ENABLE_DROMO_UPLOADINGfollows the established pattern and is properly included in the imports from the constants module.
363-363: LGTM: Proper addition to feature flag validation.The
ENABLE_DROMO_UPLOADINGconstant is correctly added to thefeatureFlagPropTypeoneOf enumeration, maintaining consistency with the existing feature flag validation pattern.src/react/src/__tests__/components/SelfServiceDromoUploader.test.jsx (8)
6-27: Well-structured mock for dromo-uploader-react component.The mock implementation effectively simulates the key functionality of the Dromo uploader:
- Properly reflects the
openstate via data attribute- Provides test buttons for cancel and results simulation
- Correctly invokes the callback functions with expected parameters
The mock design allows for comprehensive testing of the component's integration with the uploader.
29-31: Simple but effective Material-UI Button mock.The mock spreads all props and renders children, which is sufficient for testing the button behavior in this component.
33-35: Proper mocking of utility function.The
processDromoResultsfunction is correctly mocked as a Jest function, enabling verification of its invocation and arguments in the test cases.
38-44: Good test setup with proper cleanup.The test setup properly initializes mock props and includes
beforeEachcleanup to ensure test isolation.
46-50: Basic rendering test covers button presence.The test correctly verifies that the "Beta Self Service Upload" button is rendered in the document.
52-60: Effective test for uploader opening functionality.The test properly verifies that clicking the button opens the uploader by checking the
data-openattribute changes from its initial state to 'true'.
62-71: Comprehensive test for cancel functionality.The test correctly verifies the complete cancel flow: opening the uploader, clicking cancel, and confirming the uploader closes (data-open becomes 'false').
73-89: Thorough test for results handling.The test comprehensively verifies the results handling flow:
- Opens the uploader
- Simulates results submission
- Verifies
processDromoResultsis called with correct arguments- Confirms uploader closes after processing
The test validates the integration with the
processDromoResultsutility function and ensures proper state management.doc/release/RELEASE-NOTES.md (2)
12-14: Place-holders must be replaced (or dropped) before mergeSections still contain “Describe … here.” stubs.
Release notes that ship with empty bullets quickly become stale and force future readers to hunt for context elsewhere.Please either (a) fill in the actual content for Database / Schema / Code/API / Architecture / Bugfix, or (b) delete the unused headings to avoid noise.
Also applies to: 18-29
21-23: Missing Code/API changes descriptionThe PR introduces a new React component, feature flag constant, and a new
dromo-uploader-reactdependency, but this section is blank. Add at least a short sentence so automated release-note tooling (and human readers) capture the change in the correct category.⛔ Skipped due to learnings
Learnt from: VadimKovalenkoSNF PR: opensupplyhub/open-supply-hub#420 File: doc/release/RELEASE-NOTES.md:37-37 Timestamp: 2024-11-25T13:28:23.090Z Learning: When modifying unreleased API endpoints, such as refactoring the `search_after` parameter into `search_after_value` and `search_after_id` in the OpenSearch implementation, it's acceptable to leave the "What's New" section empty since the changes haven't been released to end users.Learnt from: VadimKovalenkoSNF PR: opensupplyhub/open-supply-hub#641 File: doc/release/RELEASE-NOTES.md:6-35 Timestamp: 2025-06-02T13:24:57.659Z Learning: The Open Supply Hub team keeps placeholders in release notes until code freeze, then fills in the actual content once all changes are finalized for the release.Learnt from: Innavin369 PR: opensupplyhub/open-supply-hub#415 File: src/react/src/components/ContributeForm.jsx:175-206 Timestamp: 2024-11-20T23:08:05.475Z Learning: In the `ContributeForm.jsx` React component, it's acceptable to include an empty `onClick={() => {}}` handler on a disabled `<Button>` to prevent console warnings.Learnt from: VadimKovalenkoSNF PR: opensupplyhub/open-supply-hub#420 File: doc/release/RELEASE-NOTES.md:47-54 Timestamp: 2024-11-28T06:36:47.122Z Learning: API changes and migration details are documented in a separate repository with API specifications, rather than in the release notes.Learnt from: VadimKovalenkoSNF PR: opensupplyhub/open-supply-hub#420 File: doc/release/RELEASE-NOTES.md:38-38 Timestamp: 2024-11-26T04:59:12.296Z Learning: For endpoints that haven't been released to end users, it's acceptable to document API changes under the 'Bugfix' section in the release notes.src/react/src/components/SelfServiceDromoUploader.jsx (1)
1-54: Well-structured component with good separation of concerns.The component follows React best practices with proper state management, event handlers, and prop types. The integration with the existing file input system is clean and logical.
src/react/src/__tests__/utils.tests.js (5)
88-88: LGTM: Import addition is properly placedThe
processDromoResultsimport is correctly added to the existing utility imports block.
2692-2700: LGTM: Test setup and cleanup is well-structuredThe beforeEach/afterEach setup properly creates fresh test state and cleans up mocks after each test.
2702-2712: LGTM: Edge case handling is properly testedThe tests correctly verify that the function handles undefined and empty results by taking no action.
2729-2734: LGTM: Null fileInput.current handling is properly testedThe test correctly verifies that the function handles the case when fileInput.current is null by not calling the updateFileName callback.
2673-2683: Fix the DataTransfer mock implementationThere's an inconsistency in the DataTransfer mock - the
addmethod pushes tothis.filesbut the files property getter referencesthis.items.files.Apply this fix:
global.DataTransfer = function () { this.items = { files: [], add(file) { - this.files.push(file); + this.files.push(file); }, }; Object.defineProperty(this, 'files', { - get: () => this.items.files, + get() { return this.items.files; }, }); };Wait, I need to look at this more carefully. The add method pushes to
this.filesbut the getter returnsthis.items.files. This should be:global.DataTransfer = function () { this.items = { files: [], - add(file) { - this.files.push(file); - }, }; Object.defineProperty(this, 'files', { - get: () => this.items.files, + get() { return this.items.files; }, }); + this.items.add = (file) => { + this.items.files.push(file); + }; };Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
34-36: Adjust nested-list indentation to silence markdown-lint (MD007)The linter expects two‐space indentation for nested bullets. Dropping from four to two spaces keeps the file clean without changing rendering.
-* Ensure that the following commands are included in the `post_deployment` command: - * `migrate` - * `reindex_database` +* Ensure that the following commands are included in the `post_deployment` command: + * `migrate` + * `reindex_database`
📜 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
🧠 Learnings (2)
📓 Common learnings
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#692
File: src/react/src/components/SelfServiceDromoUploader.jsx:38-39
Timestamp: 2025-07-17T11:24:39.268Z
Learning: In the Open Supply Hub project, hardcoded Dromo API credentials (license key and schema ID) in the SelfServiceDromoUploader component are acceptable for the current implementation as per project requirements.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/django/api/management/commands/enable_switches.py:15-15
Timestamp: 2024-11-20T23:12:50.325Z
Learning: In `src/django/api/management/commands/enable_switches.py`, setting `'disable_list_uploading'` to `'off'` is intentional to enable list uploading outside of the release process.
doc/release/RELEASE-NOTES.md (11)
undefined
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #641
File: doc/release/RELEASE-NOTES.md:6-35
Timestamp: 2025-06-02T13:24:57.659Z
Learning: The Open Supply Hub team keeps placeholders in release notes until code freeze, then fills in the actual content once all changes are finalized for the release.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:37-37
Timestamp: 2024-11-25T13:28:23.090Z
Learning: When modifying unreleased API endpoints, such as refactoring the search_after parameter into search_after_value and search_after_id in the OpenSearch implementation, it's acceptable to leave the "What's New" section empty since the changes haven't been released to end users.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:47-54
Timestamp: 2024-11-28T06:36:47.122Z
Learning: Endpoints that have not been enabled to end users do not require migration documentation or old vs new format examples in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:38-38
Timestamp: 2024-11-26T04:59:12.296Z
Learning: For endpoints that haven't been released to end users, it's acceptable to document API changes under the 'Bugfix' section in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #420
File: doc/release/RELEASE-NOTES.md:47-54
Timestamp: 2024-11-28T06:36:47.122Z
Learning: API changes and migration details are documented in a separate repository with API specifications, rather than in the release notes.
</retrieved_learning>
<retrieved_learning>
Learnt from: mazursasha1990
PR: #692
File: src/react/src/components/SelfServiceDromoUploader.jsx:38-39
Timestamp: 2025-07-17T11:24:39.268Z
Learning: In the Open Supply Hub project, hardcoded Dromo API credentials (license key and schema ID) in the SelfServiceDromoUploader component are acceptable for the current implementation as per project requirements.
</retrieved_learning>
<retrieved_learning>
Learnt from: vladsha-dev
PR: #490
File: deployment/environments/terraform-preprod.tfvars:17-18
Timestamp: 2025-01-29T13:36:47.477Z
Learning: In the Open Supply Hub project, the pre-prod environment is always created from scratch, so PostgreSQL version upgrade flags (rds_allow_major_version_upgrade and rds_apply_immediately) are not needed as there's no upgrade process involved. These flags should only be set for environments that are being upgraded from an existing PostgreSQL 13 instance (e.g., Staging, Production, Dev, Test).
</retrieved_learning>
<retrieved_learning>
Learnt from: Innavin369
PR: #415
File: src/react/src/components/ContributeForm.jsx:184-190
Timestamp: 2024-11-22T12:24:03.226Z
Learning: In the Open Supply Hub project, code quality improvements like adding missing prop type validations can be deferred to future improvements tasks.
</retrieved_learning>
<retrieved_learning>
Learnt from: VadimKovalenkoSNF
PR: #578
File: src/django/api/views/facility/facilities_view_set.py:905-919
Timestamp: 2025-04-11T05:17:07.809Z
Learning: In the Open Supply Hub codebase, the file extension handling in facilities_view_set.py uses a pattern where an additional dot is added before the file extension, as the extension from os.path.splitext() is expected to be used without its leading dot in some parts of the code.
</retrieved_learning>
<retrieved_learning>
Learnt from: Innavin369
PR: #431
File: src/django/api/views/v1/utils.py:95-95
Timestamp: 2024-11-28T12:55:41.805Z
Learning: When reviewing PRs focused on V1 changes, ensure comments are specific to V1 endpoints and avoid commenting on code outside of V1.
</retrieved_learning>
<retrieved_learning>
Learnt from: Innavin369
PR: #431
File: src/django/api/tests/test_v1_utils.py:147-147
Timestamp: 2024-11-28T12:54:56.277Z
Learning: When reviewing pull requests, avoid suggesting changes that are beyond the scope of the task specified in the PR objectives.
</retrieved_learning>
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-flake8-linter
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-dd-code-quality
- GitHub Check: run-integration-test-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-django-code-quality
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-fe-cov
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.env.sample (1)
5-6: Alphabetize new keys to silencedotenv-linterThe linter warns that the two new
REACT_APP_DROMO_*keys should precedeREACT_APP_GOOGLE_ANALYTICS_KEY.
Quick reorder keeps the sample file tidy and CI green.-REACT_APP_GOOGLE_ANALYTICS_KEY= -REACT_APP_DROMO_LICENSE_KEY= -REACT_APP_DROMO_SCHEMA_ID= +REACT_APP_DROMO_LICENSE_KEY= +REACT_APP_DROMO_SCHEMA_ID= +REACT_APP_GOOGLE_ANALYTICS_KEY=
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.sample(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#692
File: src/react/src/components/SelfServiceDromoUploader.jsx:38-39
Timestamp: 2025-07-17T11:24:39.268Z
Learning: In the Open Supply Hub project, hardcoded Dromo API credentials (license key and schema ID) in the SelfServiceDromoUploader component are acceptable for the current implementation as per project requirements.
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#415
File: src/django/api/management/commands/enable_switches.py:15-15
Timestamp: 2024-11-20T23:12:50.325Z
Learning: In `src/django/api/management/commands/enable_switches.py`, setting `'disable_list_uploading'` to `'off'` is intentional to enable list uploading outside of the release process.
.env.sample (1)
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#692
File: src/react/src/components/SelfServiceDromoUploader.jsx:38-39
Timestamp: 2025-07-17T11:24:39.268Z
Learning: In the Open Supply Hub project, hardcoded Dromo API credentials (license key and schema ID) in the SelfServiceDromoUploader component are acceptable for the current implementation as per project requirements.
🪛 dotenv-linter (3.3.0)
.env.sample
[warning] 5-5: [UnorderedKey] The REACT_APP_DROMO_LICENSE_KEY key should go before the REACT_APP_GOOGLE_ANALYTICS_KEY key
[warning] 6-6: [UnorderedKey] The REACT_APP_DROMO_SCHEMA_ID key should go before the REACT_APP_GOOGLE_ANALYTICS_KEY key
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: run-eslint-linter-and-prettier-formatter
- GitHub Check: run-flake8-linter
- GitHub Check: get-base-branch-countries-cov
- GitHub Check: run-dd-code-quality
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: run-fe-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: run-django-code-quality
|
OSDEV-2084 - Enable Self Service Data Upload (Dromo).
The PR enables a new self-service data upload flow using the Dromo uploader, which is implemented behind the
enable_dromo_uploadingfeature flag.