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

feat: FORMS-882 add submissionId to exports #1083

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

bcgov-citz-ccft
Copy link
Collaborator

Description

We have a Fider request for the submission ID to be added to the JSON exports. We are already exporting the confirmation ID, which is 8 characters long and taken from the submission ID. The requested change is because the submission ID is essentially unique, but the confirmation ID is much more likely to have a collision. Users who are doing ETL with the exports would greatly prefer to have the more-unique key for the submission.

Since we’re adding the submission ID to the JSON export, it should also be added to the CSV export.

https://chefs-fider.apps.silver.devops.gov.bc.ca/posts/112/data-export-missing-formsubmissionid

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have run the npm script lint on the frontend and backend
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have approval from the product owner for the contribution in this pull request

Further comments

Luckily this is already in the view, so it's just a matter of adding it to the columns pulled by the model.

@github-actions
Copy link

Coverage Report (Application)

Totals Coverage
Statements: 41.89% ( 2636 / 6292 )
Methods: 36.84% ( 326 / 885 )
Lines: 47.43% ( 1759 / 3709 )
Branches: 32.45% ( 551 / 1698 )

@github-actions
Copy link

Coverage Report (Frontend)

Totals Coverage
Statements: 68.33% ( 1107 / 1620 )
Methods: 66.11% ( 199 / 301 )
Lines: 72.44% ( 778 / 1074 )
Branches: 53.06% ( 130 / 245 )

@WalterMoar

This comment was marked as resolved.

@WalterMoar
Copy link
Collaborator

WalterMoar commented Oct 19, 2023

Thoughts on how this could break things for the users?

I suppose one possibility is that people have code to parse CSV files and they depend on column position - not a very robust way of doing things! I think adding this column would break any sort of fragile CSV-parsing process.

I posted in both the Teams and rocket.chat channels about the upcoming change and there have been no responses in the past week. I think that this possibly-breaking-change is an unlikely case and we should go ahead with this as-is.

@WalterMoar
Copy link
Collaborator

Today's my Friday so I'll merge this first thing Monday morning.

@timisenco2015 timisenco2015 merged commit e2c79e0 into master Oct 20, 2023
11 checks passed
@WalterMoar WalterMoar deleted the feat/882-export-submission-id branch October 23, 2023 16:04
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.

4 participants