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 make live validation #332

Merged
merged 4 commits into from
Sep 20, 2023
Merged

Add make live validation #332

merged 4 commits into from
Sep 20, 2023

Conversation

jamie-o-wilkinson
Copy link
Contributor

@jamie-o-wilkinson jamie-o-wilkinson commented Sep 18, 2023

What problem does this pull request solve?

Currently we have validation in forms-admin that prevents making a form live until certain conditions are met. However the api will happily make a form live if those conditions aren’t met.

This PR adds an updated task status service from forms-admin, and uses it validate the status of mandatory tasks prior to making a form live. It also adds task status data to the form data itself.

Unfortunately the submission email status depends on data in the forms-admin db, so this isn't included in the validation. Instead the submission email check is still done in forms-admin, and is combined with the incomplete_tasks and task_statuses from forms-api.

It would be great to hear other people's opinions on whether this refactor (in effect splitting the validation across forms-api and forms-admin) is preferable to duplicating the validation and removing the need to pass data back via the form object.

Trello card: https://trello.com/c/xEyusUhB

Things to consider when reviewing

  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

Move the task status service (excluding the submission email status)
from forms-admin to forms-api. This is a precursor to adding validation
to the make_live endpoint.
Add the ready_for_live, missing_sections, and task_statuses attributes
to the JSON returned when retrieving a form.
This uses the validation logic in the task status service to prevent a
form being made live if it has missing mandatory sections. If the
validation fails it returns the missing sections with a 403 Forbidden
status code.
Make each status method private, and rename missing_sections to
incomplete_tasks to be clearer about what the method does
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jamie-o-wilkinson jamie-o-wilkinson self-assigned this Sep 19, 2023
@jamie-o-wilkinson jamie-o-wilkinson changed the title WIP Add make live validation Add make live validation Sep 19, 2023
Copy link
Contributor

@thomasiles thomasiles left a comment

Choose a reason for hiding this comment

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

I think this makes things a lot simpler and it reads well. Nice one!

@jamie-o-wilkinson jamie-o-wilkinson marked this pull request as ready for review September 20, 2023 13:30
@jamie-o-wilkinson jamie-o-wilkinson added this pull request to the merge queue Sep 20, 2023
Merged via the queue into main with commit bb6c9a5 Sep 20, 2023
2 checks passed
@jamie-o-wilkinson jamie-o-wilkinson deleted the add-make-live-validation branch September 20, 2023 14:00
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