Skip to content

[OSDEV-1781] SLC form. Add clear error messages for number of workers field.#553

Merged
Innavin369 merged 14 commits intomainfrom
OSDEV-1781-fix-number-of-workers-error-message-about-min-value
Mar 13, 2025
Merged

[OSDEV-1781] SLC form. Add clear error messages for number of workers field.#553
Innavin369 merged 14 commits intomainfrom
OSDEV-1781-fix-number-of-workers-error-message-about-min-value

Conversation

@Innavin369
Copy link
Contributor

@Innavin369 Innavin369 commented Mar 10, 2025

OSDEV-1781 A clear error messages for the number of workers field has been added to the SLC form and Claimed Facility Details page.

Changes have been made on pages:

  • Production Location Information
  • Claim Facility Details
  • Claim Facility Additional Data
Screenshot 2025-03-13 at 09 51 56 Screenshot 2025-03-13 at 09 51 25 Screenshot 2025-03-13 at 09 48 32

@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 had a problem deploying to Quality Environment March 10, 2025 19:26 — with GitHub Actions Failure
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 had a problem deploying to Quality Environment March 10, 2025 19:26 — with GitHub Actions Failure
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:26 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:33 — with GitHub Actions Inactive
@barecheck
Copy link

barecheck bot commented Mar 10, 2025

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 56.14%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:34 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 10, 2025 19:34 — with GitHub Actions Inactive
@barecheck
Copy link

barecheck bot commented Mar 10, 2025

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Mar 10, 2025

Contricleaner App | Unittest test suite - Code coverage report

Total: 98.91%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@Innavin369 Innavin369 temporarily deployed to Quality Environment March 11, 2025 12:49 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 11, 2025 12:49 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 11, 2025 12:49 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 11, 2025 12:49 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 11, 2025 12:50 — with GitHub Actions Inactive
@Innavin369 Innavin369 temporarily deployed to Quality Environment March 13, 2025 09:22 — with GitHub Actions Inactive
@barecheck
Copy link

barecheck bot commented Mar 13, 2025

React App | Jest test suite - Code coverage report

Total: 33.03%

Your code coverage diff: 0.15% ▴

✅ All code changes are covered

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b09d396 and 0f634e8.

📒 Files selected for processing (4)
  • doc/release/RELEASE-NOTES.md (1 hunks)
  • src/react/src/__tests__/components/ProductionLocationInfo.test.js (4 hunks)
  • src/react/src/__tests__/utils.tests.js (3 hunks)
  • src/react/src/util/util.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/react/src/tests/components/ProductionLocationInfo.test.js
  • src/react/src/tests/utils.tests.js
  • doc/release/RELEASE-NOTES.md
⏰ 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-dd-cov
  • GitHub Check: get-base-branch-django-cov
  • GitHub Check: get-base-branch-contricleaner-cov
  • GitHub Check: get-base-branch-countries-cov

Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@VadimKovalenkoSNF
Copy link
Contributor

@Innavin369 Apply fixes from sonarqubecloud - link

roman-stolar
roman-stolar previously approved these changes Mar 13, 2025
Copy link
Contributor

@roman-stolar roman-stolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/react/src/__tests__/utils.tests.js (1)

2247-2274: Well-structured test suite for number of workers validation.

Good job creating a separate test suite for the getNumberOfWorkersValidationError function as requested in the previous review. The test cases are thorough and cover various validation scenarios including:

  • Zero values
  • Invalid ranges (min > max)
  • Values with leading zeros
  • Invalid formats

I also appreciate the integration test with isValidNumberOfWorkers that was requested.

One small suggestion to improve readability:

- expect(!isValidNumberOfWorkers('100') && getNumberOfWorkersValidationError('100')).toBe(false);
+ // For valid input, isValidNumberOfWorkers should return true and we shouldn't need an error message
+ expect(isValidNumberOfWorkers('100')).toBe(true);
+ expect(!isValidNumberOfWorkers('0-300')).toBe(true);
+ expect(getNumberOfWorkersValidationError('0-300')).toBe(expectedValueOfZeroText);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f634e8 and 7640da6.

📒 Files selected for processing (1)
  • src/react/src/__tests__/utils.tests.js (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/react/src/__tests__/utils.tests.js (1)
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#553
File: src/react/src/util/util.js:1153-1189
Timestamp: 2025-03-13T13:11:50.164Z
Learning: The `getNumberOfWorkersValidationError` function in the Open Supply Hub project is designed to be called only after a value has been determined to be invalid, not for validation checking itself. It will always return an error message since it's only used on already-invalid inputs.
⏰ 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-django-cov
  • 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-dd-cov
🔇 Additional comments (1)
src/react/src/__tests__/utils.tests.js (1)

85-86: Improvement to validation utilities for "number of workers".

The addition of these two new utility functions aligns well with the PR objective of adding clear error messages for the "number of workers" field in the SLC form and Claimed Facility Details page.

Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply fixes from sonarqubecloud - link

mazursasha1990
mazursasha1990 previously approved these changes Mar 13, 2025
Copy link
Contributor

@mazursasha1990 mazursasha1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sonarqubecloud
Copy link

Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Copy link
Contributor

@mazursasha1990 mazursasha1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants