DMPRoadmap / roadmap

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

something went wrong when exporting a plan to pdf #3340

Open StCyr opened 9 months ago

StCyr commented 9 months ago

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

v4.1.0

Expected behaviour:

roadmap shouldn't crash when I export a plan to pdf

Actual behaviour:

getting something went wrong when exporting a particular plan to pdf

Steps to reproduce:

given the following plan:

plan_65686_phase_5714_20230920T095452Z.pdf

If I try to download it in pdf format with no Optional plan components checked, or only the "unanswered questions" component checked:

image

Roadmap crashes with the infamous "something went wrong" message, and, in the logs, I can find this:

Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0] ActionView::Template::Error (undefined method `answer_hash' for nil:NilClass):
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     67:                   <% end %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     68:                   <%# case for RDA answer display %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     69:                   <% if question[:format].rda_metadata? && !blank %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     70:                     <% ah = answer.answer_hash %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     71:                     <% if ah['standards'].present? %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     72:                       <ul>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]     73:                         <% ah['standards'].each do |id, title| %>
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0]
Sep 20 11:43:56 dmponline c023693ddc24[1233]: [19647aa9-8b59-44d2-ba22-6c235c60aef0] app/views/branded/shared/export/_plan.erb:70

possible fix:

I believe the following code change fixes the issue properly:

                <%# case where question has not been answered sufficiently to display%>
---                <% if @show_unanswered && blank %>
+++                <% if @show_sections_questions && @show_unanswered && blank %>
                  <br>
                  <p><%= _('Question not answered.') -%></p>
                  <br><br>
---                <% else %>
+++                <% elsif @show_sections_questions %>

                  <%# case where Question has options %>
                  <% if options.present? && options.any? %>
                    <ul>
                      <% options.each do |opt| %>
                        <li><%= opt.text %></li>
                      <% end %>
                    </ul>
                  <% end %>
                  <%# case for RDA answer display %>
                  <% if question[:format].rda_metadata? && !blank %>
                    <% ah = answer.answer_hash %>
                    <% if ah['standards'].present? %>
                      <ul>
                        <% ah['standards'].each do |id, title| %>
                          <li><%= title %></li>
                        <% end %>
                      </ul>
                    <% end %>
                    <p><%= sanitize ah['text'] %></p>
                    <br>
                  <%# case for displaying comments OR text %> 
                  <% elsif !blank %> 
                    <%= sanitize answer&.text %> 
                    <br><br> 
                  <% end %> 
                <% end %>
nicolasfranck commented 9 months ago

Variable blank is created on line https://github.com/DMPRoadmap/roadmap/blob/main/app/views/shared/export/_plan.erb#L40, but within an "if" statement. If that statement isn't executed, that variable does not exist, and what is worse: variable answer does not exist.

Better move these lines :

               <div class="question">
+                <% answer = @plan.answer(question[:id], false) %>
+                <% blank = answer.present? ? answer.blank? : true %>
                 <% if @show_sections_questions%>
-                  <% answer = @plan.answer(question[:id], false) %>
-                  <% blank = answer.present? ? answer.blank? : true %>