Forms: Rename `mandatory` prop to `isMandatory`

[skjnldsv]

https://github.com/nextcloud/forms/blob/53b6851a79984ba24a873e3423bb15b3fb386002/src/mixins/QuestionMixin.js#L48-L54

To be consistent with other variables, because it looks weird here
https://github.com/nextcloud/forms/blob/d9ff7a180a045404b8eddf1fc5a9631f6bf0527e/src/components/Questions/QuestionDropdown.vue#L192-L203

[Chartman123]

Should the term 'mandatory' be kept here as it has been renamed in the frontend from 'mandatory' to 'required' with #464?

[jotoeri]

Hm.... generally i wouldn't mind if it is named required, mandatory or isMandatory....
I just wonder if it is worth to do that change... I think we would need a new Api-Version as well as a new Major-Version, then, no? Only renaming on frontend-code but leaving the backend would probably produce more chaos on the code... 🤔

So i'd probably shift that to Forms v3, maybe we change the API then anyways...? 🤔 @skjnldsv ?

[skjnldsv]

No, it's only on the vue code.
For consistency. No api change

[jotoeri]

Well mandatory is a question-property, which goes down to DB and is given to the frontend via API.
https://github.com/nextcloud/forms/blob/master/docs/DataStructure.md#question

If we just change it on the frontend-code, we will need some mapping somewhere and the consistency problem just gets shifted...

[skjnldsv]

Right, we can always just add a :is-mandatory="question.mandatory" somewhere around here https://github.com/nextcloud/forms/blob/53b6851a79984ba24a873e3423bb15b3fb386002/src/views/Create.vue#L96

But is it worth it.
Damn, we should have done that sooner. Really dislike inconsistencies ^^'

[Chartman123]

Sorry for bringing up that discussion 😉 I just wanted to prepare a new PR since the other one hasn't been updated since a few months... But I think it's better to wait a little for the decision to be made.

[skjnldsv]

@jotoeri though, is there anyone out there already using the forms api?
Let's change now that no one uses it?

[jotoeri]

🤷‍♂️ I just know we had some issues in this direction already... And i don't see the change that urgent.

However - what just came into my mind: We could rename it everywhere on our code, and copy it on public API to a second prop of the old naming just before the API-Response is sent. It's a bit hacky, but such we don't break the API, can release the change with the next minor and just state the mandatory prop as deprecated and to be removed with v3.0.

However - if we do that step we should probably now properly think about, how we name it, if it's isMandatory or isRequired as @Chartman123 proposed. I'll have to look into the code the next days to finally give my opinion on this, but isRequired might be the better version if we anyways rename...^^ 😉

(EDIT: Just to have it noted somewhere API v1.1 its anyways then... 😉 👍 )

[skjnldsv]

I'll let you decide then, no problem :)

[jotoeri]

So. I think from code-view isRequired is the way to go. This is closest to the HTML-Property and we don't have any other naming dependency.
The wording of the user-info An asterisk indicates mandatory questions i'd probably not change for now, buth that's details then and might change anyways in some design-process. 🤷‍♂️

@Chartman123 are you still motivated to have a look into? 😉

[Chartman123]

I'll have another look at it and see if it's within my coding skills... I think I could at least do it partially.

[jotoeri]

Yeah, I'd say just give it a try and ask if you have questions... 🙂

[Chartman123]

First question: I've already created a fork to my own repository. Should I keep it there? I've been added to the Nextcloud Github organization recently, so I think I could also create a new branch directly here... What's the best way to go?

[jotoeri]

Yeah, you can push here, if you want. Mainly that makes it a bit easier for us/others to checkout the code, if wanted. 👍
I personally push here usually and just sometimes use a fork to store some backup before doing very large rebases or to store some dirty tests or so...

[Chartman123]

Yes, that's what I thought about, too 🙂 I just did a first push to a new branch refactor-mandatory. Could you please have a look if I did it correctly?

[jotoeri]

The branch is available here, so looks good, yes. 👍
However i also spotted a pitfall already on your commit. Wanna open a PR already, so we can discuss such stuff there then? 😉