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

Disallow changing of form element type #1257

Open
Tracked by #1465 ...
mahalakshme opened this issue Jun 18, 2024 · 4 comments
Open
Tracked by #1465 ...

Disallow changing of form element type #1257

mahalakshme opened this issue Jun 18, 2024 · 4 comments
Assignees

Comments

@mahalakshme
Copy link
Contributor

mahalakshme commented Jun 18, 2024

Issue:

During testing or when self-service organisations change the form element configuration(Single to Multi-select, repeatable to non-repeatable) of question type(Coded or QuestionGroup) without knowledge, the app breaks.

Analysis:

Should not allow to change them since anyways 'app designers' can always create new form elements and migrate the data to it.

AC:

  • When Type selection(multi/single select) on coded concept changed or when 'question group' concept made from Repeatable to non-repeatable(or vice-versa) show message under Repeatable or Type element(in coded concept) stating 'Cannot modify this directly. To proceed with this change, contact support team if there exists important data for this Question, else delete this question and create a new one and associate it with a different new concept.'
Screenshot 2024-08-06 at 12 21 58 PM Screenshot 2024-08-06 at 12 21 19 PM

Example mockup for showing error message:

Image

  • The above should happen when editing an already existing/saved form, not when changing the type or repeatable before saving the form for the first time.

Out of scope:

Introducing new data types that will support switching between types.

Analysis notes:

  • Should we do this as bg job? No, since difficult to migrate client data
  • or just disable, - No, might cause confusion
  • Handling in product(without migration) doesn't seem to be a good idea since we need to do at rule execution, displaying in both mobile and web app - many places to handle - so No
  • disallow only for prod - No, confusion bcoz inconsistent across orgs
  • allow when there is no data - No, if allowed, then need to handle during bundle upload since the destination org, may have data
  • so just disallow with proper error message(try with new form element/concept) on the workaround

Suggestion:

  • One thing I see we can do is, store multi-select, single-select, RQG, QG etc., in the same format ie., array or whatever, and use an attribute to determine the logic what to do
  • Need not migrate the existing data, instead when we see a object in place of array, treat it accordingly.
  • We cant by default have everything as multiselect since it depends on the business use case.
  • concrete egs why it was needed to change
  • we 'll not scale now
@vinayvenu
Copy link
Member

If no forms have been filled, we can allow the change.

@mahalakshme mahalakshme changed the title Prevent changing of form element type Support changing of form element type Sep 17, 2024
@mahalakshme mahalakshme changed the title Support changing of form element type Disallow changing of form element type Sep 19, 2024
@1t5j0y 1t5j0y self-assigned this Sep 23, 2024
1t5j0y added a commit to avniproject/avni-server that referenced this issue Sep 24, 2024
…es on form save

Single <-> Multi select for multi select capable concept data types
Repeatable <-> Non repeatable for QuestionGroups
1t5j0y added a commit to avniproject/avni-server that referenced this issue Sep 24, 2024
@petmongrels
Copy link
Contributor

UX Inputs

  • in cannot use the same concept twice, name of concept will be required by user to resolve the problem.
  • in cannot change the type, we should provide the name of the form element with group name to pin-point the issue

@mahalakshme
Copy link
Contributor Author

mahalakshme commented Sep 27, 2024

@petmongrels in cannot change the type, we should provide the name of the form element with group name - if the error message is going to be near the place where change has happened, pin-pointing might not be needed. AC is to mention it nearby and hence didn't mention in the message.

I understand the message is different from AC. But applies.

1t5j0y added a commit to avniproject/avni-server that referenced this issue Sep 27, 2024
@1t5j0y
Copy link
Contributor

1t5j0y commented Sep 27, 2024

Fixed form validation server side error messages to include more information of the offending elements. Kept webapp error message as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA Ready
Development

No branches or pull requests

4 participants