form validation for unique option names in draft

[semla]

Adds custom validator for unique option names in the DraftpollPage

fix #181

[CLAassistant]

All committers have signed the CLA.

[mensch72]

Hi! Thanks for the start!

I testet it and it almost works. When I first enter an option "A" and then try another "A", this is correctly invalid. But if I add "A", then "B", and then go back to "A" and rename it also to "B", this passes as valid but should be invalid.

I believe the problem is that the validator is currently given the entries of pd, which however get updated with some delay. I think the validator should rather be given the list of current values of all the option_name_xx form fields, which can be received as this.formGroup.get('option_name'+i).getValue().

In addition, we should then also add a validation message in the dict at the end of draftpoll.page.ts and a corresponding entry in en.json.

[semla]

Thanks for the quick feedback! Makes sense, but when I do unique_name_validator(this.formGroup.get('option_name'+i).value) there is an error in the console: Error: Uncaught (in promise): TypeError: this.formGroup.get(...) is null I guess because the form is not yet set up. I think a possible solution is to get all the names inside the Validator by referencing the form directly, but haven't gotten it to work yet. Another solution I can think of is to subscribe and keep a local copy of the names entered.

[mensch72]

what if you remove the .value from this call and rather put it into the validator code itself?

Am 18. September 2022 09:18:17 MESZ schrieb semla @.***>:

Thanks for the quick feedback! Makes sense, but when I do unique_name_validator(this.formGroup.get('option_name'+i).value) there is an error in the console: Error: Uncaught (in promise): TypeError: this.formGroup.get(...) is null I guess because the form is not yet set up. I think a possible solution is to get all the names inside the Validator by referencing the form directly, but haven't gotten it to work yet. Another solution would be to subscribe and keep a local copy of the names, that should be easy

-- Reply to this email directly or view it on GitHub: https://github.com/pik-gane/vodle/pull/188#issuecomment-1250208982 You are receiving this because you commented.

Message ID: @.***>

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[semla]

3033906 takes the values from the form instead using async validator, what do you think?

[mensch72]

hm, my other suggestion did not work? i will test your version later today :-)

Am 18. September 2022 19:22:50 MESZ schrieb semla @.***>:

3033906 takes the values from the form instead using async validator, what do you think?

-- Reply to this email directly or view it on GitHub: https://github.com/pik-gane/vodle/pull/188#issuecomment-1250351584 You are receiving this because you commented.

Message ID: @.***>

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[semla]

hm, my other suggestion did not work?

iiuc the error is that you can't do formGroup.get('xx') in the call, since it is the control xx that is being added

[mensch72]

so then why not pass only the formgroup object in the call and have the gets in the validator code? at the time the validator is invoked, the formgroup elements exist, right?

[mensch72]

your last pushed version works fine in my test, so let's leave it this way :-)

[mensch72]

Thank you @semla for your work, I have now learned about Observables for the first time (vodle is my first TS project). One last question: Could you sign your commits? If not, I must bypass my own branch protection rule, but I would do it if you cannot sign the commits :-)

[semla]

Will do, I might add a test to the draft-component also. Observables are a handful but quite useful, especially in Angular since it is used quite a lot. Cheers

[semla]

Now it is just one commit and signed

[mensch72]

for some reason it still says "Merging is blocked The base branch requires all commits to be signed. Learn more about signing commits." but i will bypass that check now :-)

[semla]

yeah had some reading up to do on signing, especially old commits, but the new one #0ec2f74 should be signed? can there be some other github setting preventing?

[mensch72]

don't know, probably it was because the PR contained older, unsigned commits. anyway, i simply bypassed it. thank you again!

Am 21. September 2022 06:52:25 MESZ schrieb semla @.***>:

yeah had some reading up to do on signing, especially old commits, but the new one #0ec2f74 should be signed? can there be some other github setting preventing?

-- Reply to this email directly or view it on GitHub: https://github.com/pik-gane/vodle/pull/188#issuecomment-1253202666 You are receiving this because you modified the open/close state.

Message ID: @.***>

-- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

[semla]

Your are welcome!