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

Vacms 16208 police testing #1822

Merged
merged 7 commits into from
Dec 6, 2023
Merged

Vacms 16208 police testing #1822

merged 7 commits into from
Dec 6, 2023

Conversation

eselkin
Copy link
Contributor

@eselkin eselkin commented Dec 5, 2023

Summary

  • Added unit testing to verify conversion of CSV data to JSON per expected format
  • Sitewide Facilities

Related issue(s)

Testing done

  • Adds testing for expected format of CSV (requested from Axon)
  • Adds unit test with missing files (missing contacts data)
  • Adds unit test with present data and expects conversion
  • Adds mock data in CSV format expected from Axon.

Screenshots

No UI changes

What areas of the site does it impact?

Solely unit testing is added.

Acceptance criteria

unit tests pass

Quality Assurance & Testing

  • I added unit tests for conversion of expected Axon CSV data.
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (not necessary)
  • Screenshot of the developed feature is added (no screenshot)
  • Accessibility testing has been performed

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user (no changes for any path)

Requested Feedback

Check that tests pass.

@va-vfs-bot va-vfs-bot requested a review from a team December 5, 2023 01:19
Copy link
Collaborator

@va-vfs-bot va-vfs-bot left a comment

Choose a reason for hiding this comment

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

ESLint is disabled

vets-website uses ESLint to help enforce code quality. In most situations we would like ESLint to remain enabled.

What you can do

See if the code can be refactored to avoid disabling ESLint, or wait for a VSP review.

});
it('should process files', async () => {
const client = getCurlClient({
'drupal-user': '[email protected]',

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "[email protected]" is used as
authorization header
.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test curl client doesn't actually log in anywhere, so even if this email is eventually defunct at some point, this test will still pass.

src/site/stages/build/drupal/tests/police/download.unit.spec.js Dismissed Show dismissed Hide dismissed
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/VACMS-16208-police-testing December 5, 2023 01:25 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/VACMS-16208-police-testing December 5, 2023 03:12 Inactive
@eselkin eselkin marked this pull request as ready for review December 5, 2023 16:30
@eselkin eselkin requested review from a team as code owners December 5, 2023 16:30
@jilladams jilladams requested a review from maxx1128 December 5, 2023 16:59
@jilladams
Copy link
Contributor

@eselkin this PR auto-tagged Public Websites front end, but not Facilities front-end. Could you add a rule to content-build codeowners file that makes Facilities the point of contact for the Police path? That can go into this PR, or a separate one, whatever is easier.

@eselkin
Copy link
Contributor Author

eselkin commented Dec 5, 2023

@jilladams It doesn't want to seem to catch that directory. Already we should be requested because we changed src/site/stages/build/drupal since the vaPoliceData directory is under there and we are not requested. I added it to be specific but GitHub doesn't request the review.

@va-vfs-bot va-vfs-bot temporarily deployed to master/main/VACMS-16208-police-testing December 5, 2023 20:19 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/VACMS-16208-police-testing December 6, 2023 18:57 Inactive
Evidently I cannot add spaces without annoying the linter
@eselkin eselkin merged commit f507a45 into main Dec 6, 2023
22 checks passed
@eselkin eselkin deleted the VACMS-16208-police-testing branch December 6, 2023 22:20
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.

6 participants