Skip to content

Submit data contributions to the new /oed/contributions endpoint - #89

Open
matthewberry wants to merge 2 commits into
mainfrom
mjberry/fix-contribution-form
Open

matthewberry wants to merge 2 commits into
mainfrom
mjberry/fix-contribution-form

Conversation

@matthewberry

@matthewberry matthewberry commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #88. Every submission from the About page's data contribution form has been failing. The form uploaded to POST /oed-contributions/upload, and the backend now rejects any bucket on that route that isn't a job type (400 Unknown bucket).

This PR points the form at the dedicated endpoint added in moleculemaker/mmli-backend#158, POST /oed/contributions.

Depends on moleculemaker/mmli-backend#158. Don't deploy this until the backend change is live. Until then, the form gets a 404 instead of a 400, so it's still broken.

Changes

  • about-page.component.ts calls OpenEnzymeDBService.submitContributionOedContributionsPost(file, email, licenseAccepted, rightsConfirmed).
    • It sends the original file. Before, the file was renamed to <email>_<filename>.
    • It now sends both checkbox answers. Before, they were checked in the browser and then dropped.
    • The checkboxes are non-binary p-checkboxes, so a checked one holds an array (confirmed in the installed PrimeNG 17.18.11, updateModel). That's why the booleans are !!value?.length.
  • API client: I added /oed/contributions to openapi/mmli-backend.openapi.json by hand, copied from the backend branch's generated OpenAPI. Staging doesn't serve it yet, and npm run openapi:sync would pull in a lot of unrelated changes (the committed spec has 18 paths). Regenerating with the pinned generator (6.6.0) added only the new service and its two index entries (now openEnzymeDB.service.ts, after the backend renamed the tag from OED to OpenEnzymeDB).

Testing

  • about-page.component.spec.ts: the form submits the original File, the email and true, true, and the status becomes submitted. A backend error sets the status to error. The spec also now provides the router and service, so "should create" runs.
  • The karma build is already broken on main: three unrelated specs don't compile (molecule-name-validator.directive.spec.ts, smiles-validator.directive.spec.ts, entity-ecnumber.component.spec.ts), and that stops every ng test run. I didn't fix them here. I ran this spec with a temporary tsconfig that included only it: 7/7 passed. Of the 4 tests added with the review fixes, the rejected-file and me@lab tests fail on the old logic.
  • ng build succeeds.
  • Not tested end to end: the spec mocks OpenEnzymeDBService, and nothing has been sent to a running backend.

Review fixes (635cd35)

  • The file picker is capped at 50 MB, the backend's limit, and the form takes the file from event.currentFiles. PrimeNG still lists a file that failed its size check in event.files, so the cap alone didn't keep an oversized file out of the form.
  • The email field requires a dot in the domain. Validators.email accepts me@lab, which the backend rejects with a 422. Reserved domains like .local/.test still get past the form and receive the generic error.

The About page form uploaded to POST /oed-contributions/upload, which the
backend has rejected with 400 "Unknown bucket" since that route was
restricted to job types, so every contribution failed. Send it to the
dedicated endpoint added in moleculemaker/mmli-backend#158 instead.

The form now sends the original file with the email and both agreements
as fields, instead of prefixing the email to the file name and dropping
the checkboxes.

The endpoint is added to the committed OpenAPI spec by hand: staging
does not serve it yet, and a full re-sync would pull in unrelated drift.

Fixes #88
@matthewberry

Copy link
Copy Markdown
Contributor Author

Review note: the form's email validation is looser than the backend's.

Validators.email (about-page.component.ts:53) accepts me@lab, but the backend's EmailStr rejects it. I checked this in the backend test image: me@lab was rejected and me@example.org was accepted. So the submit button enables, the backend returns 422, and the user sees the generic "please ensure that all fields are filled out correctly" message. That's the same gap as the 1 GB picker vs 50 MB backend limit in the follow-up. Either add a Validators.pattern that requires a dot in the domain, or add it to that follow-up.

Related, posted on moleculemaker/mmli-backend#158: lowering maxFileSize to 50 MB here would also stop accidental 1 GB uploads from being fully received by the backend pod before it returns 413.

- Cap the file picker at 50 MB (the backend's limit) and take the file from
  event.currentFiles. PrimeNG's onSelect still lists a file that failed its
  size/type check in event.files, so maxFileSize alone did not keep an oversized
  file out of the form.
- Require a dot in the email domain. Validators.email accepts me@lab, which the
  backend's EmailStr rejects with a 422.
- Regenerate the client for the backend's renamed tag (OED -> OpenEnzymeDB):
  OEDService / oED.service.ts become OpenEnzymeDBService / openEnzymeDB.service.ts.
@matthewberry

Copy link
Copy Markdown
Contributor Author

Confirmed. Angular 18.2.13's EMAIL_REGEXP makes the dotted-domain group optional, and pydantic 2.10.4's EmailStr (validate_email(..., check_deliverability=False)) rejects me@lab with "It should have a period." Fixed in 635cd35 with Validators.pattern(/@.+\..+$/). This narrows the gap but doesn't close it: email-validator also rejects reserved domains like me@lab.local, me@host.test and me@lab.123, and those would still get the generic error.

The same commit fixes the 1 GB picker. It sets maxFileSize to 50 MB and reads event.currentFiles instead of event.files. PrimeNG still includes a file that fails the size check in event.files, so the cap alone wouldn't have kept it out of the form. A rejected file now clears the form control, and basic mode shows PrimeNG's size message. It also picks up the backend's tag rename (OEDService → OpenEnzymeDBService).

Tests: about-page.component.spec.ts passes 7/7 using a spec-only tsconfig, since the three unrelated specs still don't compile on main. The two new regression tests fail against the old logic. ng build succeeds.

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.

Data contribution form always fails: backend rejects the 'oed-contributions' upload bucket (400 Unknown bucket)

1 participant