Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor determination javascript #6191

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jrmhaig
Copy link
Contributor

@jrmhaig jrmhaig commented Oct 27, 2023

What

Rewrite the javascript for calculating determinations to follow the GovUK Javascript style guide.

Ticket

N/A

Why

Some motivation is written in docs/javascript_restructure.md

In summary, the Javascript used in CCCD is written in inconsistent styles and have dependencies that are starting to make it difficult to keep the application up to date.

How

The determination calculation on the claim summary page for case workers was in app/webpack/javascripts/modules/case_worker/claims/DeterminationCalculator.js. The new version is now in app/webpack/javascripts/modules/determination.js and tests are in app/webpack/javascripts/modules/determination_spec.mjs.

The GovUK govuk-frontend module is enabled by adding the class govuk-frontend-supported to the <body>. This convention is for the new Determination class. The class is initialised in app/webpack/packs/govuk-frontend.js in the same way as govuk-frontend modules are initialised by require('govuk-frontend').initAll().

@jrmhaig jrmhaig requested review from a team as code owners October 27, 2023 12:00
@jrmhaig jrmhaig force-pushed the refactor_determination_javascript branch 6 times, most recently from 9ca53a3 to 32c0493 Compare October 30, 2023 12:40
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jrmhaig jrmhaig force-pushed the refactor_determination_javascript branch from 32c0493 to df855cd Compare December 13, 2023 16:07
@jrmhaig jrmhaig force-pushed the refactor_determination_javascript branch 5 times, most recently from a02e9ff to 3ef1141 Compare January 5, 2024 18:40
}

cleanNumber = (element) => {
element.value = element.value.replaceAll(/[^\d.]/g, '').replace(/^(\d*\.\d\d).*$/, '$1')
Copy link
Contributor Author

@jrmhaig jrmhaig Jan 8, 2024

Choose a reason for hiding this comment

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

There is a better way to do this by splitting the string on the first . to get the integer and mantissa parts and then remove all of the non-numeric characters from each part. As it is, a string like 12.3.4 does not get fixed.

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
26.1% Duplication on New Code

See analysis details on SonarCloud

@jrmhaig jrmhaig force-pushed the refactor_determination_javascript branch from 9dfa988 to 4afb768 Compare March 4, 2024 14:21
jrmhaig added 7 commits March 8, 2024 10:10
For some reason Response is not being recognised even though it is part of
Javascript (https://caniuse.com/?search=Response). This is possibly due to out
of date dependencies preventing ES6 coding standards being checkd.

It is added to the list of globals to allow `yarn standard` to pass. It may be
possible to remove this after refactoring.
@jrmhaig jrmhaig force-pushed the refactor_determination_javascript branch from 4afb768 to 7a33d4b Compare March 8, 2024 10:10
Copy link

sonarqubecloud bot commented Mar 8, 2024

Quality Gate Passed Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
26.1% Duplication on New Code

See analysis details on SonarCloud

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.

1 participant