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

Integrate sds with launcher #49

Merged
merged 17 commits into from
Jun 30, 2023
Merged

Conversation

katie-gardner
Copy link
Contributor

@katie-gardner katie-gardner commented Jun 27, 2023

What is the context of this PR?

This PR is the launcher component of the ticket to Integrate SDS with Launcher. When a schema is selected, the survey_id of that schema is retrieved and returned along with the metadata so that if the metadata also contains survey_id, then it will be pre-populated with the survey_id of the survey (to make life easier for devs)

The metadata survey_id can be changed to anything, and when both survey_id and period_id are populated fields in the metadata, and when either of them change, a new call is made to the mock endpoint in the corresponding runner PR to fetch the SDS datasets and use them to fill the dropdown list.

Changes

Adjusted launch.go to replace the metadataHandler with a surveyDataHandler. This is because we need the survey_id from the schema, to pre-populate any survey_id that might be found in metadata, and it was not enough to return the metadata alone to render the page.

Additionally in launch.go added a similar supplementaryDataHandler to intercept any requests to the supplementary-data url in the front-end and make the request to the mock end point with proper handling

The actual fetching of supplementary datasets is done in surveys.go in a similar style to other fetches and using the DatasetMetadata struct to model the response, as per the sds api documentation

launch.html has been refactored to improve the way fetches were being done, make reusable components for the various input fields and whenever the schema changes and metadata is populated, check for a survey id and period id in the metadata and fetch supplementary datasets if both can be found.

If supplementary datasets are found, the supplementary data div is displayed and populated with the selected dataset

How to review

  1. Clone and run the launcher PR with go run launch.go
  2. Run the supporting services for runner with make dev-compose-up as normal (delete the launcher container if that is built despite launcher already being run)
  3. Launch the mock SDS from a terminal in runnerpython -m scripts.mock_sds_endpoint
  4. Launch runner as normal.
  5. Open launcher in chrome
  6. Select V2 Launch Pattern and the schema test_supplementary_data
  7. Survey Metadata should load as normal, with survey id 123 and a new sds_dataset_id dropdown with only one option.
  8. The first and only SDS dataset metadata should have loaded and displayed below.
  9. The survey should be launchable.
  10. Open launcher again
  11. Select V2 Launch Pattern and the schema test_supplementary_data_with_multiple_datasets
  12. Survey Metadata should load as normal, with survey id 456 and 2 sds_dataset_ids in the sds_dataset_id dropdown. The first one will auto-select.
  13. Swap between the given options and the Dataset Metadata section should update to reflect the metadata.
  14. Launch the survey with both options to verify it launches correctly.
  15. Open launcher again
  16. Select V2 Launch Pattern and either test_supplementary_data or test_supplementary_data_with_multiple_datasets.
  17. Enter a random number for the survey id in the survey metadata section.
  18. The Dataset Metadata section should disappear and the sds_dataset_id dropdown should clear of options, and the Launch Survey button should not be available, as the survey requires an sds_dataset_id value.
  19. In step 17, using the id of the schema you did not load, will load the available datasets for that survey but it will be an invalid dataset for the selected survey and thus crash on launch.

How to review

  • Read through the trello card and check that the requirements have been met, and that the changes are suitable
  • As this is a dev tool, consider if this implementation offers the most helpful functionality it can for working with pre-pop tickets

Copy link
Contributor

@MebinAbraham MebinAbraham left a comment

Choose a reason for hiding this comment

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

LGTM

launch.go Outdated Show resolved Hide resolved
surveys/surveys.go Show resolved Hide resolved
Copy link
Contributor

@MebinAbraham MebinAbraham left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@berroar berroar left a comment

Choose a reason for hiding this comment

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

Minor, but when I open launcher the Supplementary Data is always displayed e.g.
Screenshot 2023-06-30 at 07 58 44
Do we want this to be hidden until a survey is selected?

@katie-gardner
Copy link
Contributor Author

Minor, but when I open launcher the Supplementary Data is always displayed e.g. Screenshot 2023-06-30 at 07 58 44 Do we want this to be hidden until a survey is selected?

I can't replicate this on the browsers I've tried 🤔 should be covered by the change to the css - is it still broken after a chrome update if there is one?

@MebinAbraham
Copy link
Contributor

Minor, but when I open launcher the Supplementary Data is always displayed e.g. Screenshot 2023-06-30 at 07 58 44 Do we want this to be hidden until a survey is selected?

I can't replicate this on the browsers I've tried 🤔 should be covered by the change to the css - is it still broken after a chrome update if there is one?

You need to clear the browser cache and it would fix this @berroar

Copy link
Contributor

@petechd petechd left a comment

Choose a reason for hiding this comment

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

I can't find anything except some whitespace and what @berroar mentioned above.

@katie-gardner katie-gardner merged commit 474c432 into main Jun 30, 2023
@katie-gardner katie-gardner deleted the integrate-sds-with-launcher branch June 30, 2023 08:56
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.

5 participants