Closed noamoss closed 4 years ago
@akariv it might be connected to #47, which was reported by me during preparations.
The root cause of this issue is captured in #50 .
Depends on #50
@akariv I will take care of the script level, but I think this is a scenario to throw an error when processing the YAML. I (i.e. script editors) can not know by heart all valid variable names. Would you like to open a separate issue for that, or take care of it here?
This is something that ofer added to his script validator that (hopefully) he will incorporate into the build process.
On Wed, Oct 2, 2019 at 8:19 PM noamoss notifications@github.com wrote:
@akariv https://github.com/akariv I will take care of the script level, but I think this is a scenario to throw an error when processing the YAML. I (i.e. script editors) can not know by heart all valid variable names. Would you like to open a separate issue for that, or take care of it here?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hasadna/reportit-agent/issues/49?email_source=notifications&email_token=AACAY5KQIFHJPSWGEOUPIOLQMTJZTA5CNFSM4I3DEKFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAFQRFI#issuecomment-537594005, or mute the thread https://github.com/notifications/unsubscribe-auth/AACAY5MIF7ISQGVLATHHBMTQMTJZTANCNFSM4I3DEKFA .
@akariv Ofer will add validations in the UI level (meaning, he will not let editor add a non-existing variable name) - but as you can see #50 is about a temporary/local variable, but someone might still use local variable names in a wrong name or touch the YAML manually.
In other words, I still think we need this kind of validation also in the build phase - to prevent the site from getting into an error state just because of a mistype. don't you think?
The problem only happens for non-temporary variables, as only those get sent to the server (and possibly cause an error)
On Fri, Dec 13, 2019 at 11:02 PM noamoss notifications@github.com wrote:
Assigned #49 https://github.com/hasadna/reportit-agent/issues/49 to @akariv https://github.com/akariv.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/hasadna/reportit-agent/issues/49?email_source=notifications&email_token=AACAY5OJK4E77Z65UL4EHCLQYPZ7BA5CNFSM4I3DEKFKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOVPI6IDY#event-2882659343, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACAY5MXZRLNUDPCJ4MB3KLQYPZ7BANCNFSM4I3DEKFA .
and housing_land_type
does not start with an underscore, so it's not a temporary/local variable by definition.
It should be easy for the validator to point out that it's an invalid variable name.
(following #30) real case description, contact Saroj for further details:
https://admin.equality.org.il/#/report/151
This is a real problem, not because of a practical inconvenience, but because of the risk to lose (overwrite) saved data: