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

Fix for issue #355: Don't update form name from drafts if published version exists #357

Closed
wants to merge 7 commits into from

Conversation

ktuite
Copy link
Member

@ktuite ktuite commented Apr 16, 2021

This PR addresses issue #355 and updating form titles at the right times in the work flow.

Expected behavior, for which there are now tests:

  • If no published version, update form title with draft title
    • needs a conditional check
  • If published version, update title on next publish
    • needs to get title from somewhere (reading xml stream of from form def)
      • How to add something to form def?

This PR has gone through a couple phases:

Phase 1: Parse form name from FormDef XML on publish

First, I set up some checks in the draft upload part of the code so the new title will only get applied from a draft to the overall form if there is no published versions.

For transferring the title from the form from a draft on publish, I first tried an approach of reading the title directly from the form def xml. It had to wait for that xml to get loaded and parsed, so I don't know if it's a great approach (but also it's ok if publish is a little slow). It also had some clunky async/await code that seemed a little fragile, so I moved on to the next phase.

Phase 2: Add name to FormDef

Since each form XML definition has a title (or null title) that can change with each version, it makes a lot of sense to store that name here. (In the XML, the form name is called title, but everywhere in the code, including the database column name, this field is called name for consistency.)

I made a migration that adds the name column and backfills it with title information read from each FormDef xml.

I then use that FormDef.name to update the Form.name at the right places in the code.

Phase 3: Remove Form.name and just use the appropriate FormDef.name (not reflected in this PR yet)

Copying over the form name is confusing and redundant and in an effort to normalize the data, I'm working on removing Form.name and using the appropriate FormDef.name, since the form_def table is joined in any query about forms already.

@ktuite ktuite requested review from issa-tseng and lognaturel April 16, 2021 00:26
@ktuite ktuite force-pushed the ktuite/form-def-form-titles branch from 679df70 to eeaa7d1 Compare April 22, 2021 18:19
@ktuite ktuite force-pushed the ktuite/form-def-form-titles branch from eeaa7d1 to a767baa Compare April 22, 2021 22:12
@ktuite
Copy link
Member Author

ktuite commented Apr 22, 2021

Current response for /v1/projects/<proj_id>/forms/<form_id> when the form only exists as a draft looks like this:

name is filled in (from Form.name) but title is null (from FormDef.title but all form def information is withheld because the form is only a draft)

  {
      "createdAt": "2021-04-16T00:03:03.593Z",
      "draftToken": null,
      "enketoId": null,
      "enketoOnceId": null,
      "hash": null,
      "keyId": null,
      "name": "Titled Form",
      "projectId": 3,
      "publishedAt": null,
      "sha": null,
      "sha256": null,
      "state": "open",
      "title": null,
      "updatedAt": "2021-04-21T18:17:32.709Z",
      "version": null,
      "xmlFormId": "titled-form"
  }

@ktuite
Copy link
Member Author

ktuite commented Apr 27, 2021

Closing this and going with PR #359 to fully remove name from Form and use name stored on FormDef instead.

@ktuite ktuite closed this Apr 27, 2021
@ktuite ktuite deleted the ktuite/form-def-form-titles branch April 27, 2021 22:45
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.

1 participant