bedatadriven / activityinfo-R

ActivityInfo R Language Client
https://www.activityinfo.org/support/docs/R/
18 stars 12 forks source link

BUG: adding new field (`addFormField()`) with same exact field ID as another field doesn't throw errors // modifying existing fields with schema functions #82

Closed Ryo-N7 closed 1 year ago

Ryo-N7 commented 1 year ago

as i was trying to test migrateFieldData() , i was also thinking about the fact that we don't have a specific function to modify existing fields...?

so instead what I tried to do was create "new" fields or more like modifying existing ones by setting the field ID the same as an already existing field with the addFormField() functions but this simply creates another field with the SAME ID in the schema.

both fields have ID: ccwhu70lfrpd3z6e

overwrite-field-same-id

i can even successfully push that updated form schema to the server succs-copy-form

it shows up in the UI, and you can grab the fields through the "export" button but the fields show up with the SAME field ID, which "should" theoretically cause problems but it isn't somehow

both fields have ID: ccwhu70lfrpd3z6e

copy-field-ui same-field-id-ui

Ryo-N7 commented 1 year ago

Also, instead of wrapping the formschema functions with addFormField()

i instead also tried to modify existing fields by simply piping through the _FieldSchema() functions to the existing schema but i keep bumping into errors:

step-newdesc-text

while there is a "code" argument as in the function docs, it doesn't show up in the auto-complete:

code-not-pop

same with key, required and hideInTable, and the function keeps throwing an error if you don't include them.

i imagine this is because normally you're supposed to use addFieldForm() which automatically fills those in for you?

even after i fill in every single argument manually, i still get another error:

modify-eixsting

this probably means that I'm not using this properly // we don't have a function to modify existing fields in a form schema?

Ryo-N7 commented 1 year ago

while deleteFormField() allows you to reference the field you want to delete with ANY one of label/code/id perhaps this shouldn't be the case?

because in cases like above where duplicate fields can be created with the same label/code/id that can cause problems?

i was thinking about how for any new modifyField() function we create, we need to be more specific and have the user supply the field ID as that ID "should" be UNIQUE? or will that be too restrictive and we should simply do what deleteFormField() does currently?

Ryo-N7 commented 1 year ago
Ryo-N7 commented 1 year ago

yep, it's quite imperative that we at least make sure we get those sanity checks in because just now in the user interface, i tried to delete one of the duplicate fields from the 'form design' view aaaand it delete them both, at the same time!

even before that, when i clicked on one of the fields in the 'form design' view, it would open both fields up so we could already see the problems there

nickdickinson commented 1 year ago

This should now be addressed including a few more functions:Adding addition form checks to addForm(), updateFormSchema(), and validateFormSchema()

nickdickinson commented 1 year ago

Sanity checks are in place now so I believe it is unlikely, without manual editing of the list object, to create a situation where the IDs and/or the codes are the same.