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 url schema launch #62

Merged
merged 12 commits into from
Dec 20, 2023
Merged

Fix url schema launch #62

merged 12 commits into from
Dec 20, 2023

Conversation

rmccar
Copy link
Contributor

@rmccar rmccar commented Dec 15, 2023

What is the context of this PR?

This makes the change to only send the schema url to runner when launching via a schema url

Also fixes the alignment of the arrow icon next to the load previous value button and tidys up the code a bit.

How to review

  • Run this launcher branch locally, see that the arrow icon is now aligned
  • Launch a schema from a url and see that the schema launches. This is a url you can use to test that: https://raw.githubusercontent.com/ONSdigital/eq-questionnaire-schemas/main/schemas/business/en/1_0005.json
  • Use the quick launch facility to launch a schema and see that the schema launches correctly without error. This is a url you can use to test that: http://localhost:8000/quick-launch?url=https://raw.githubusercontent.com/ONSdigital/eq-questionnaire-schemas/main/schemas/business/en/1_0005.json

@rmccar rmccar force-pushed the fix-url-schema-launch branch from 1be4d18 to 3bb02d8 Compare December 15, 2023 13:05
@rmccar rmccar marked this pull request as ready for review December 18, 2023 14:01
@rmccar
Copy link
Contributor Author

rmccar commented Dec 18, 2023

Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

Works for v2 launching 👍
Doesn't work for v1, the launch errors because runner is expecting eq_id and form_type or the schema name, but in this case it doesn't have it

I think possibly the change for the v1 launch pattern should be undone? Since the validation in runner for v1 is ok with it the way it was, just v2 that doesn't work

@rmccar rmccar force-pushed the fix-url-schema-launch branch from 45fe402 to 85553df Compare December 19, 2023 10:48
@rmccar
Copy link
Contributor Author

rmccar commented Dec 19, 2023

Works for v2 launching 👍 Doesn't work for v1, the launch errors because runner is expecting eq_id and form_type or the schema name, but in this case it doesn't have it

I think possibly the change for the v1 launch pattern should be undone? Since the validation in runner for v1 is ok with it the way it was, just v2 that doesn't work

Fixed this now 👍

Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

All seems to be working for me 👍

@berroar
Copy link
Contributor

berroar commented Dec 20, 2023

Works for v2 launching 👍 Doesn't work for v1, the launch errors because runner is expecting eq_id and form_type or the schema name, but in this case it doesn't have it
I think possibly the change for the v1 launch pattern should be undone? Since the validation in runner for v1 is ok with it the way it was, just v2 that doesn't work

Fixed this now 👍

I still seem to get this error, when launching a test schema by URL in v1 (was using test_checkbox) 🤔

@rmccar
Copy link
Contributor Author

rmccar commented Dec 20, 2023

Works for v2 launching 👍 Doesn't work for v1, the launch errors because runner is expecting eq_id and form_type or the schema name, but in this case it doesn't have it
I think possibly the change for the v1 launch pattern should be undone? Since the validation in runner for v1 is ok with it the way it was, just v2 that doesn't work

Fixed this now 👍

I still seem to get this error, when launching a test schema by URL in v1 (was using test_checkbox) 🤔

Im not seeing this, what are the steps are you doing to get the error?

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.

Tested, LGTM 👍

Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

All working as expected now 👍

@rmccar rmccar merged commit 03ae413 into main Dec 20, 2023
@rmccar rmccar deleted the fix-url-schema-launch branch December 20, 2023 13:52
@berroar berroar added the bug Something isn't working label Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants