Skip to content

Submissions.createNew(): return less data#1608

Merged
alxndrsn merged 3 commits intogetodk:masterfrom
alxndrsn:joingo-returns
Sep 9, 2025
Merged

Submissions.createNew(): return less data#1608
alxndrsn merged 3 commits intogetodk:masterfrom
alxndrsn:joingo-returns

Conversation

@alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Sep 6, 2025

  • inline _defInsert()
  • invert INSERT query to replace nextval() with standard key generation
  • reduce data returned from PostgreSQL which NodeJS already has
  • reformat main query

Follow-up to #1606

Having read _defInsert() enough times to understand it, it seemed helpful to rewrite it. I assume the reason why the submission_def was being inserted before the submission was to allow use of _defInsert() by both Submissions.createNew() and Submissions.createVersion(), which is no longer happening.

What has been done to verify that this works as intended?

CI.

Why is this the best possible solution? Were any other approaches considered?

  • This PR inverts the insertion order of submission and submission_def to rely on PostgreSQL to assign the submission's id. This is orthogonal to decreasing the data passed between PostgreSQL and NodeJS.
  • This PR could be changed to return query formatting to match the original more closely. To me this makes understanding the query significantly harder.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

No effect

Does this change require updates to the API documentation? If so, please update docs/api.yaml as part of this PR.

No.

Before submitting this PR, please make sure you have:

  • run make test and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

* line `_defInsert()`
* invert INSERT query to replace `nextval()` with standard key generation
* reduce data returned from PostgreSQL which NodeJS already has
* reformat main query
@alxndrsn alxndrsn marked this pull request as ready for review September 6, 2025 14:52
Copy link
Contributor

@brontolosone brontolosone left a comment

Choose a reason for hiding this comment

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

LGTM with G for Great

@alxndrsn alxndrsn merged commit 6b15bb6 into getodk:master Sep 9, 2025
6 checks passed
@alxndrsn alxndrsn deleted the joingo-returns branch September 9, 2025 15: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.

3 participants