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

New slash syntax in parameters is not supported #169

Closed
MattiSG opened this issue Aug 9, 2018 · 6 comments
Closed

New slash syntax in parameters is not supported #169

MattiSG opened this issue Aug 9, 2018 · 6 comments

Comments

@MattiSG
Copy link
Member

MattiSG commented Aug 9, 2018

As shown in https://saucelabs.com/jobs/56e91565aad14feb98de97c7862b9e19, the new slash syntax for parameters introduced in some new version of the web API breaks the legislation explorer. It prevents the router from loading the page altogether.

This is a major problem for the Legislation Explorer as its integration tests are currently based on the French API, and @openfisca/france-admin have deployed this latest version live, which means that all tests will now fail, preventing merges.

This new slash syntax should be supported, as it is apparently the new way to navigate parameters.
This also underlines the need to isolate tests from any live environment.

@Anna-Livia
Copy link
Contributor

The problem seems to be due to the /parameters page --> This parameter list defines the routing of the LegExp. When we changed the key from "cotsoc.gen.smic_h_b" to "cotsoc/gen/smic_h_b", the routing module looks for a parameter called "cotsoc" and doesn't find it.

  • Step one : Roll back the openfisca-core dependency used in api/v21 so the LegExpl functions again.
  • Step Two : Create api/v22 with the latest core/france dependencies
  • Step Three : Open a PR to update the Leg Exp to be compatible with the new Core.

I hope this will help :)

@fpagnoux
Copy link
Member

fpagnoux commented Aug 9, 2018

The problem seems to be due to the /parameters page

Humm, all the side-effect on /parameters were not really intented, ar ot least not thought trough.

You're right, let's roll back and see what we do next.

@Anna-Livia
Copy link
Contributor

Roll back is done, and there is now a v22 api with the new core available for tests :)

@Anna-Livia
Copy link
Contributor

@MattiSG I feel that we can close the issue.

This also underlines the need to isolate tests from any live environment.

I agree, I opened an issue on the subject so we can discuss how to go about it : #170

@fpagnoux
Copy link
Member

@MattiSG I feel that we can close the issue.

Currently, the legislation explorer is still not compatible with the latest version of Core. Until this is fixed, this issue should probably stay open.

@fpagnoux
Copy link
Member

Fixed by openfisca/openfisca-core#710

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

No branches or pull requests

3 participants