roadmap icon indicating copy to clipboard operation
roadmap copied to clipboard

DMP Assistant Feature - allow phase-only download

Open pengyin-shan opened this issue 2 years ago • 8 comments

Changes proposed in this PR:

  1. Disable Capybara::Webmock.start since Webmock complains about the use of this method (start is not there anymore I believe)

  2. New Feature from DMP Assistant: allow phase-only download for plans:

Now users can choose to download a single phase or all phases of the plan from the Download page (all formats except json). A dropdown is provided to users on UI to select: Screen Shot 2022-10-03 at 1 29 57 PM Then users could download the coversheet (if selected) and the phase they choose.

For CSV now, we allow user to download single phase only, no 'All Phases' avaible. We will add option to download all phases as a zip file later. If you want, you can disable this feature for csv for now by modifying the code (lin 25) in app/javascript/src/plans/download.js

pengyin-shan avatar Oct 03 '22 17:10 pengyin-shan

Thank you @pengyin-shan. I'll try to look at it tomorrow. We have our dev server running with rails 6.

martaribeiro avatar Oct 04 '22 17:10 martaribeiro

Thank you @briri and @martaribeiro ! Please let me know if you have any questions.

pengyin-shan avatar Oct 04 '22 17:10 pengyin-shan

I checked it locally. It works in rails 6 and in a 3 Phase plan :)

Great feature. Thank you

Thank you @martaribeiro ! It also works on single-phase plans on your side right? (like the simple plans that don't have separate phase)

pengyin-shan avatar Oct 07 '22 15:10 pengyin-shan

Screenshot 2022-10-08 at 11 18 15 So when there is only one phase, the drop down doesn't display, which makes sense. But the PDF file does not include any questions or answers... @pengyin-shan I didn't check this before. I only tried the plan with multiple phases, and it did work when I selected all phases or a single phase. Not sure why it doesn't work for a single phase plan, as this was ok before. I should say I can see the plan details in the PDF, just not the questions or answers

martaribeiro avatar Oct 08 '22 10:10 martaribeiro

Screenshot 2022-10-08 at 11 18 15

So when there is only one phase, the drop down doesn't display, which makes sense. But the PDF file does not include any questions or answers... @pengyin-shan I didn't check this before. I only tried the plan with multiple phases, and it did work when I selected all phases or a single phase. Not sure why it doesn't work for a single phase plan, as this was ok before. I should say I can see the plan details in the PDF, just not the questions or answers

@martaribeiro sorry I thought the new pushes will be automatically notified to reviewers, but seems not...can you try again? I added one more commit

pengyin-shan avatar Oct 11 '22 16:10 pengyin-shan

@pengyin-shan I'll try later tonight or tomorrow morning if it's ok.

martaribeiro avatar Oct 11 '22 17:10 martaribeiro

@pengyin-shan I'll try later tonight or tomorrow morning if it's ok.

No problem! Please take your time and thank you a lot!!

pengyin-shan avatar Oct 11 '22 17:10 pengyin-shan

@pengyin-shan I tried again and the same problem happens. When there is only one phase and I download a pdf, it's empty. No questions. I attached some screen grabs. Not sure what is happening

Screenshot 2022-10-12 at 13 44 36 Screenshot 2022-10-12 at 13 48 22 Screenshot 2022-10-12 at 13 15 11

martaribeiro avatar Oct 12 '22 13:10 martaribeiro

I get the same result as @martaribeiro here. The issue lies with the following line in the plan_exports_controller.rb

@selected_phase = @plan.phases.order('phases.updated_at DESC')
                                             .detect { |p| p.visibility_allowed?(@plan) }

The visibility_allowed? function returns false unless 50% of the Plan's questions have been answered. I think it's safe to remove that .detect call.

To replicate, try generating a PDF for a plan (based on a template with a single phase) that does not have more than 50% of questions answered

briri avatar Nov 29 '22 17:11 briri

I get the same result as @martaribeiro here. The issue lies with the following line in the plan_exports_controller.rb

@selected_phase = @plan.phases.order('phases.updated_at DESC')
                                             .detect { |p| p.visibility_allowed?(@plan) }

The visibility_allowed? function returns false unless 50% of the Plan's questions have been answered. I think it's safe to remove that .detect call.

To replicate, try generating a PDF for a plan (based on a template with a single phase) that does not have more than 50% of questions answered

Thank you @briri and @martaribeiro ! I see why I cannot duplicate the behavior you have. In DMP Assistant we set default_percentage_answered = 0 in _dmproadmap.rb instead of 50.

I'll remove .detect and push again

pengyin-shan avatar Nov 29 '22 18:11 pengyin-shan

@briri sorry I want to confirm again before I made modifications since this seems to be a different feature change without concerning the 'All Phases' feature there is here or not.

We used to have all the 'uncompleted' questions & answers not downloadable. 'Uncompleted' or 'Completed' status is based on the default_percentage_answered setting.

By removing this line, we won't care about 'Uncompleted' or 'Completed' anymore. Any 'Uncompleted' questions and answers will be downloadable.

If this is what we need, let me know and I'll include this in the PR description.

pengyin-shan avatar Nov 29 '22 20:11 pengyin-shan

Hmm. That's a good question @pengyin-shan

I guess we still would want to hide uncompleted answers. Instead of making the change I suggested, can you update the logic in the view so that it outputs a message like _('No questions have been answered for this DMP.')

That way It will at least inform the user about why the PDF is empty. I'm open to other suggestions/options :)

briri avatar Nov 29 '22 22:11 briri

Hmm. That's a good question @pengyin-shan

I guess we still would want to hide uncompleted answers. Instead of making the change I suggested, can you update the logic in the view so that it outputs a message like _('No questions have been answered for this DMP.')

That way It will at least inform the user about why the PDF is empty. I'm open to other suggestions/options :)

From the DMP Assistant perspective, we have seen users feel confused about default_percentage_answered to define answered/unanswered questions, so we just made it to 0.

In order to avoid such confusion for other roadmap users, how about _('Rest of the DMP hasn't been completed yet.')? user can still see 'completed' questions & answers.

pengyin-shan avatar Nov 29 '22 22:11 pengyin-shan

@mariapraetzellis @dsisu can you recommend an appropriate message for this? The scenario is when someone exports their plan and has not answered any questions but has also selected not to display the question text. The resulting document is empty which is confusing and causes people to think there is an error.

briri avatar Jan 23 '23 16:01 briri

@martaribeiro : I verified this PR again by 1) config.x.plans.default_percentage_answered = 0 in_dmproadmap.rb then download 2) check 'unanswered questions' when downloading the plan, and the unanswered part will show. So your problem could link to this setting instead of the multi-phase download feature.

@briri : we discussed adding messages to let users know that since they haven't completed the plan, the rest of the content won't show. Unfortunately, I only have one day left so I don't have time to work on this feature. However, I created #3295 as a future reference.

Other than these, this PR is ready to go. It allows users to download both single-phase and in PDF, TEXT, and DOCX formats. Note that 1) CSV file can only download a single phase instead of all phases 2) JSON doesn't have this feature yet.

If you want to wait and discuss more of the single-phase download feature before merging this PR, please talk to Omar or Shiloh.

pengyin-shan avatar Feb 27 '23 20:02 pengyin-shan

I think this is ok to merge but lets discuss in the next team meeting

briri avatar Mar 23 '23 19:03 briri

going to merge this. I have tested it manually and It works as-is for single phase plans and I like the phase selector when the template has multiple phases.

briri avatar Mar 23 '23 21:03 briri