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

Add authority bulk create (backend & frontend) #3708

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

mbklein
Copy link
Contributor

@mbklein mbklein commented Dec 5, 2023

Summary

Implement bulk creation of local authorities

Specific Changes in this PR

  • Add /api/authority_records/bulk_create route for submission of CSV
  • Add Bulk Add button to Local Authorities dashboard

Version bump required by the PR

See Semantic Versioning 2.0.0 for help discerning which is required.

  • Patch
  • Minor
  • Major

Steps to Test

  1. Create a CSV file with two columns (with headers): label and hint and several rows of new authorities. Feel free to include labels that already exist to test duplicate handling.
  2. Log in as a user with access to the Local Authorities dashboard
  3. Navigate to the Local Authorities dashboard
  4. Click the Bulk Add button
  5. Drag 'n' drop or click-to-select your CSV file.
  6. Click Upload

The modal popup should close, and your browser should download a CSV file containing the results (labels, hints, IDs, and an indication of whether the line item was created or if it was a dupe).

You can do a "bad data" test by:

  • submitting a CSV file with too many columns or headers with the wrong names
  • submitting a file that isn't a CSV at all

Bad data should cause a redirect right back to the local authorities page. Due to the nature of how this direct POST works, displaying an error message would take a lot more work that might not be worth it.

🚀 Deployment Notes

Note - if you check any of these boxes go to the (always open) main <- staging PR and add detailed notes and instructions to help out others who may end up deploying your changes to production

  • Backward compatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
  • Backwards-incompatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
    • New backend route
  • Requires data migration
  • Requires database triggers disabled during deployment/migration
  • Requires reindex
  • Terraform changes
    • Adds/requires new or changed Terraform variables
  • Pipeline configuration changes (requires mix meadow.pipeline.setup run)
  • Requires new variable added to miscellany
  • Specific deployment synchronization instructions with other apps/API's
  • Other specific instructions/tasks

Tested/Verified

  • End users/stakeholders

@mbklein mbklein force-pushed the 3994-4122-import-local-authorities branch 2 times, most recently from 9ac1f7b to 83c2acb Compare December 5, 2023 21:37
@mbklein mbklein requested review from bmquinn, adamjarling and kdid and removed request for adamjarling December 5, 2023 22:32
@kdid
Copy link
Contributor

kdid commented Dec 6, 2023

@mbklein - I'm getting a 400 error. I did do a mix deps.get and a mix assets.install.

This is all I saw in the log:

module=Phoenix.Logger [debug] Processing with MeadowWeb.AuthorityRecordsController.bulk_create/2
  Parameters: %{}
  Pipelines: [:api]
module=Meadow.Accounts.User [debug] User ksd927 found in cache
module=Phoenix.Logger [debug] Converted error Phoenix.ActionClauseError to 400 response
Screenshot 2023-12-06 at 7 15 39 AM

@mbklein mbklein force-pushed the 3994-4122-import-local-authorities branch from 83c2acb to 2bc25b7 Compare December 6, 2023 17:03
@mbklein mbklein changed the base branch from 4366-import-local-authorities to deploy/staging December 6, 2023 17:06
@mbklein mbklein merged commit f737261 into deploy/staging Dec 6, 2023
4 checks passed
@mbklein mbklein deleted the 3994-4122-import-local-authorities branch December 6, 2023 18:49
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.

2 participants