DMPRoadmap / roadmap

DCC/UC3 collaboration for a data management planning tool
MIT License
104 stars 110 forks source link

Conditional questions causing plans to disapear #3163

Closed mariapraetzellis closed 2 years ago

mariapraetzellis commented 2 years ago

Plans that include a conditional question are disappearing. For example, a user can successfully create a plan from a template including a conditional question, but when they go back to view the plan, the plan will disappear from the page.

We temporarily fixed this issue by removing the conditional question from the template, but I believe this bug will be affecting all plans that include conditional questions.

johnpinto1 commented 2 years ago

@raycarrick-ed This seems to be related to issue I noticed when dealing with a bug for Maastricht. The plan sections container continues to have style="display:none". Will investigate.

johnpinto1 commented 2 years ago

@mariapraetzellis, @briri Not yet identified cause, but working on it. As I have an example which fails.

johnpinto1 commented 2 years ago

@briri Just trying out things. I think the issue is in app/views/phases/edit.html.erb Then clicking the "Write Plan" tab results

Before (code change) Selection_004

Selection_005

After minor code change to app/views/phases/edit.html.erb based on app/views/contributors/index.html.erb

<% title "#{plan.title} - Write plan" %>
<div class="row">
  <div class="col-md-12">
    <h1><%= plan.title %></h1>
  </div>
</div>

<% content_for :plan_tab_body do %>
<div class="row">
  <div class="col-md-12">
    <%= render partial: 'phases/edit_plan_answers',
               locals: {
                 plan: plan,
                 phase: phase,
                 answers: answers,
                 readonly: readonly,
                 base_template_org: base_template_org,
                 guidance_presenter: guidance_presenter,
               } %>
  </div>
</div>
<% end %>

<%= render partial: "plans/navigation", locals: { plan: plan } %>

Results in Selection_003

The content still has "display:none" but navigation tabs appear. Not sure why this display:none persists. Selection_006

johnpinto1 commented 2 years ago

Just proved in broken case cause the rendering causes everything in content_for to be surrounded by a display:none block even though content exists. Not sure if it is a timing issue.

<%= render partial: 'phases/edit_plan_answers',
               locals: {
                 plan: plan,
                 phase: phase,
                 answers: answers,
                 readonly: readonly,
                 base_template_org: base_template_org,
                 guidance_presenter: guidance_presenter,
               } %>
briri commented 2 years ago

Thanks for digging into this one @johnpinto1. Based on what you've found so far, I suspect that there is something in the JS that is hiding the entire panel instead of just the conditional questions it should be targeting.

This bit attempts to hide questions if they've met the condition: https://github.com/DMPRoadmap/roadmap/blob/7cac43502862d013cd509c3486bfe4e61fd619b5/app/javascript/src/answers/conditions.js#L5

That calls a function that finds the closes class="row". https://github.com/DMPRoadmap/roadmap/blob/7cac43502862d013cd509c3486bfe4e61fd619b5/app/javascript/src/utils/sectionUpdate.js#L22

One of the recent updates might have changed the class definitions on this page. There must be a class name that's more appropriate that we can use here that will give us the correct dom element. Note that closest searches the dom tree in an upward direction aka parents of the element, so another option would be to switch it to .find('ror') which searches the child elements. or .siblings('row') which would search all adjacent elements. If not, you could always add one. Be sure to check if that getQuestionDiv function is used anywhere else and make sure a change there won't break something else on the page.

johnpinto1 commented 2 years ago

Thanks @briri. A cursory test shows commenting out hide questions fixes issue.

Will tomorrow dig deeper as suggested in your comment.

pherterich commented 2 years ago

I think @raycarrick-ed added this to the latest release, so I will close out here.

johnpinto1 commented 2 years ago

@pherterich I can't see it in https://github.com/DigitalCurationCentre/roadmap/commits/deploy/dmponline?before=99d780716fca202f19a6dcb7da40ce62edb52e76+35&branch=deploy%2Fdmponline&qualified_name=refs%2Fheads%2Fdeploy%2Fdmponline It is possible has been patched by @raycarrick

pherterich commented 2 years ago

It is patched and part of release 3.1.1 proper