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

Allow conditional fields / question logic / ordering #1772

Closed
sjDCC opened this issue Aug 2, 2018 · 41 comments
Closed

Allow conditional fields / question logic / ordering #1772

sjDCC opened this issue Aug 2, 2018 · 41 comments
Assignees
Labels
admin questions templates user group suggestions made at user group meetings

Comments

@sjDCC
Copy link
Contributor

sjDCC commented Aug 2, 2018

The question of conditional fields has been raised as a feature request several times (i.e. If no, skip to q6). I'd like to consider adding it to the list and scoping out how much work it is to implement.

Thoughts @stephaniesimms @briri @xsrust @Bodacious

Has there been demand for this in the USA?

Mandatory questions were requested in the past too but there had been issues with this in DMPTool so we may want to keep that out of scope

@sjDCC sjDCC added the questions label Aug 2, 2018
@stephaniesimms
Copy link

I can imagine that a handful of specialized users (Smithsonian, USGS) would make use of this kind of functionality as they did with mandatory questions, but these are the sorts of overly complex features that we sought to remove from our previous version and deemed out of scope for a simple wizard tool. If there is some simple, low effort way to achieve this I'm happy to explore it though.

@rosiehigman
Copy link

I was about to add a similar issue requesting this, if you do decide to go ahead and are looking for institutions to test than Manchester would be really keen to do this.

@sjDCC
Copy link
Contributor Author

sjDCC commented Aug 17, 2018

Great, thanks @rosiehigman If you could add details about the kind of functionality you'd be looking for (is it literally just the 'if no, skip to question 6' use case or more?) that would be really helpful.

@rosiehigman
Copy link

Hi @sjDCC, I think the primary use case would be if users only selected 'no personal or sensitive data' on Q9 of our Manchester section of each plans then it would skip to Q15 (saving researchers lots of time!). I suppose this is slightly more complicated in that it is a question where they can select multiple options and we would only want it to skip questions if this was the only option selected.

If this was available we would probably implement it in a couple of places on our templates and may want to nest some conditionality e.g. If yes, skip to question 6, if yes on Q6 then skip to Q8, if no on Q6 then skip to Q7.

@rosiehigman
Copy link

Hi @sjDCC and @stephaniesimms, do you have any idea if this is likely to be developed/what the time frame for this will be? This has been requested again from several schools and a senior committee this morning. Thank you!

@martateperek
Copy link

@sjDCC & @stephaniesimms - I fully second @rosiehigman plea for conditional questions - this is a huge deal breaker for us (and other colleagues in the Netherlands I spoke with)

@sjDCC sjDCC added admin templates user group suggestions made at user group meetings labels Apr 16, 2019
@sjDCC
Copy link
Contributor Author

sjDCC commented Apr 16, 2019

The need for conditional questions was validated in the NL & UK user groups. Some notes from sessions below to help scope this out:

  • Mainly needed to allow user to skip a bunch of questions e.g. ethics or security. This could be in response to a Y/N question or on selection of a particular option. See Manchester template question 9
  • Admins want notices on screen e.g. "Now go to…" so we need an accompanying field to enter instructions
  • Triggers also desired e.g. on data volume – if it exceeds a certain amount, email uni...
  • Make it clear to user that the trigger is about to occur e.g. if notifying uni of data volume so they can get in touch. Shouldn't be a surprise to end user. Again a field to enter notice on screen would help.

We propose it as a discrete feature development for this Summer. It was the top priority item

@sjDCC
Copy link
Contributor Author

sjDCC commented Sep 19, 2019

This was demonstrated at user group. Suggested changes were as follows:

  • Allow emails to be configured to be sent to multiple teams (i.e. same email to ethics and RDM) so allow multiple addresses to be put in 'to' field
  • Allow admin to tick box to send same email to user too so they are aware of
  • Add option to remove whole section not just per question. Login needs to be linked to and overwritten by changes to question selection
  • Allow admin to set delay on issuing notification emails by time e.g. 1 hour, 5 hours, 1 day etc
  • Allow admins to specify hover-over text so users know when email notifications will be issued by selecting certain values

@mariapraetzellis
Copy link

@raycarrick-ed will work on this and can wait until the next release.

@mariapraetzellis
Copy link

Ray continuing to work on this during the week and will write up notes on functionally issues and bugs so we can make decisions about how the feature is intended to work.

@raycarrick-ed
Copy link
Contributor

I suspect the logic of this is not what we want in the long term but at this point, it is probably better to go with what we have at the moment and then get people's reactions to it.

Issues I can see are:

  • it only allows for removal of issues which forces you into contortions if you want to add in questions (but that may be a less common use case)
  • this is just the conditional functionality. Not the wehooks, though there is some code for that there.
  • there is a workflow issue around creating the template. The model here is: create all questions, then go back and add in conditions. If try and do these in a more integrated way rather than as two separate phases, you can get into some very awkward work flows e.g. create some options, add some conditions then go back and add a new option or change an option. It won't show up in the conditions until it is saved and when you save it the whole thing is now collapsed and you have to go in and open it up again to get to the point where you can add a condition. To create a slicker workflow would involve rewriting the whole thing which I think we will do in the end but before then, it'd be better to be more certain about what users actually want.
  • when you create a template you can set an option as a default and you can also create a condition on that option to remove other questions. What should happen when the plan is first displayed? Show everything and wait for the user to save the defaulted answer and then remove the conditional questions or not display the conditional questions because they are "eliminated" by the defaulted answer? Currently it does the former but that's a bit weird. This is because when you make something the default, it just means that it is chosen by default when the plan is displayed. But the removal of questions is based on saved answers which initially there are none of. This behaviour could be changed but,agian, do we know for sure what is wanted.
  • the code uses a mix of styles where some of the work takes place in JS and some by calling end points to return partials. This would be good to regularise.
  • the handling of defaults can seem strange here too. When we display the plan the default is ticked but it's not saved so if the user doesn't save it then it will revert to the non-default value when they save the whole plan.

@mariapraetzellis
Copy link

mariapraetzellis commented Mar 5, 2020

A few notes from testing:

  1. Button for Conditional Questions looks grayed out but does function. Shouldn't it be orange?
    screenshot-dmponline-test dcc ac uk-2020 03 04-16_18_29

  2. Looked for @sjDCC previously described bug with 404s when removing a condition and it looks like that has been fixed w/ Ray's last rebuild. I was able to remove the conditions w/ out a problem.

  3. I added a notification condition and got a 404 after trying to save it. Weird though because it was only a 404 w/in the specific section of the window in the template:
    screenshot-dmponline-test dcc ac uk-2020 03 04-16_33_55

@sjDCC
Copy link
Contributor Author

sjDCC commented Mar 5, 2020

We did a whole series of UAT together and found several issues:

  • It seems to be working on checkboxes but not radio buttons when editing an existing template
  • Re-editing a preset condition generates a 404 and jumps user into historic version of template which can’t be edited
  • Attempting to re-edit preset conditions a second time seemed to save (notification at top of page suggested this worked) yet new values didn’t save
  • No emails were generated. Maybe a Glasgow email issue
  • When creating conditions on a new template, none of the values saved correctly. Mixed up which triggers were set on which options.
  • Template export seemed to work - yay!

@xsrust
Copy link
Contributor

xsrust commented Mar 16, 2020

It looks like all the funky-behavior may be down to template versioning.

I've fixed that logic, but to re-test, you'll need to re-set the conditions and re-publish the templates.
After that, It should work as expected, but it would be good to check that funder templates are functioning as expected by adding some conditions, publishing, customizing, and creating a plan.

@mariapraetzellis
Copy link

I tried to add a new condition to an existing template but the Add Conditions button did not work & I noticed it is still grayed out.

screenshot-dmponline-test dcc ac uk-2020 03 16-15_47_35

@raycarrick-ed
Copy link
Contributor

I wonder if this is a process issue. Once the options have been created, you need to save the question before you can then go back in to add the conditions. This screenshot looks like it is in the state where it has created but not saved. After the save of the question the button should then be active.

The issue with the color is a different thing not connected. That will probably be something not set up right in the deployment.

@magdalenadrafiova
Copy link

I did UAT based on our last points raised by Sarah and created a new template Test template for conditional questions NWO.
It worked well but the emails were still not not generated.
screenshot-dmponline-test dcc ac uk-2020 03 17-13_40_50
Also the button appeared the right colour before the conditions were added but once the conditions were added it was greyed out but it was possible to add further conditions see below:
screenshot-dmponline-test dcc ac uk-2020 03 17-13_42_05

@mariapraetzellis
Copy link

@raycarrick-ed will add mouseover text to the Add Conditions button in order to prompt users to save a new question before they Add Conditions.

@mariapraetzellis
Copy link

@raycarrick-ed will check the log for emails to make sure they are getting through

@raycarrick-ed
Copy link
Contributor

More explicit tooltip added to make logic clearer in: 78e52fe

@raycarrick-ed
Copy link
Contributor

acbbdff

removes a funky question option reordering that was being done. I think it was probably intended to catch the case where someone added a condition and changed an option at the same time but it was reassigning the question option ids based on whatever order they came out of the DB. So if you had 3 options "Yes, No & Maybe" and set a condition on "Yes" it might, in the process of saving reassign the condition to be triggered by "Maybe". Hadn't spotted this before.
Also cleans up a couple of other minor issues like when displaying the question options, the order was random and the email subject wasn't displaying properly.

@mariapraetzellis
Copy link

mariapraetzellis commented Mar 19, 2020

I was able to successfully add a condition (after saving first this time!). I didn't see a tooltip on the Add Condition button prompting users to save their question first. Was that deployed?

screenshot-dmponline-test dcc ac uk-2020 03 19-09_51_55

Also noticed the Add Condition button is not changing color.

Tested notifications and successfully received the email. @briri We'll need to change the email text to include DMPTool info.

@raycarrick-ed
Copy link
Contributor

There's no tooltip at that point because you have already added conditions. The tooltip is there at the point where you have created the question options but not yet saved. See image here...

AddConditionsTooltip

@mariapraetzellis
Copy link

Thanks for the clarification, Ray. It looks good to me; passing to @magdalenadrafiova & @sjDCC for final UAT.

@sjDCC
Copy link
Contributor Author

sjDCC commented Mar 24, 2020

Thanks @raycarrick-ed

Have tested this on an existing template and all worked well. Correct actions were followed based on conditions set and email notifications received. It means template versioning is working. I also saw the tool tip.

Will try to break it in next half hour before noon telecon but bodes well...

@sjDCC
Copy link
Contributor Author

sjDCC commented Mar 24, 2020

OK, on testing further I think I have found three bugs. @magdalenadrafiova @mariapraetzellis can you try to examine these further:

  1. When I edit an existing template with radio buttons or checkboxes, the values for these are somtimes empty. Has something overwritten the data in existing templates? At first I thought I'd forgotten to fill them in but the same occurred on a couple of existing templates from different orgs (University of Manchester Generic template and Edinburgh's UoE Data Management Plan) though others (e.g. ZonMw) had info filled in
    missing-values

This and issue 2 seem to be about versioning. Perhaps conditions aren't replicating through properly in cloning process?

  1. When editing a previously published template with conditions, I hit a 404 when editing the condition. See screebgrab below and other example at https://dmponline-test.dcc.ac.uk/org_admin/templates/6254/phases/7466/sections/44696/questions/110361
    testing
    When this happened, the save didn't change and I was returned to a historical version of the template which is uneditable. If I re-navigated to the edit view, I could make the adjustment and save successfully. The 404 only occurs on the first attempt to edit a pre-existing condition

  2. I added a combination condition (i.e. if it is both windy and sunny, remove X). Single conditions seem to render the correct behaviour on the UI but not combinations. This requires further testing as I only trialled once so it could be user error
    combination

@raycarrick-ed
Copy link
Contributor

Number 1, loss of options isn't to do with the conditions, looking at the versions of that template. The versions were lost some time back (Jan 2019) so I'd gues it was a bug at that pointwhich has since been fixed. Worth double checking by creating a new template with options and then adding conditions but I think it's ok now.

@raycarrick-ed
Copy link
Contributor

Number 3 was a duff comparison between the conditions set and the options chosen. They are now sorted first to ensure consistent ordering:

f36c7c7

@raycarrick-ed
Copy link
Contributor

Number 2 here turned out to be really horrible. When we update a published template, we create a new version. At that point all of the ids in the condition we have got back from the form may be wrong. So the condition has an option list of question option ids which may have changed in the versioning and so need to be rewritten in the params so we can do the update properly. Likewise the question numbers stored in the remove question array may have changed and they could be any question in the template. So this fixes up both of those.

74efb6b

to test:

  • create a template with a couple of questions
  • create a couple of conditions
  • publish the template
  • change one of the conditions
  • save the change
    got back in an look at the template. If it looks ok then all should be good.

@raycarrick-ed
Copy link
Contributor

Pulled this back into review because it's not just a small bug fix. There's quite a bit of change in here so in needs more eyes on it.

@magdalenadrafiova
Copy link

worked well for me

@sjDCC
Copy link
Contributor Author

sjDCC commented Mar 31, 2020

Yay! Have just tested every eventuality and it all worked beautifully for me too. Thanks for all the work @raycarrick-ed

With great pleasure I declare this closed!

@sjDCC sjDCC closed this as completed Mar 31, 2020
@magdalenadrafiova
Copy link

YEY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin questions templates user group suggestions made at user group meetings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants