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.

Sign up for GitHub

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

Issue202: Add contributor to the DMP downloaded files #206

Merged
merged 5 commits into from
May 24, 2022
Merged

Issue202: Add contributor to the DMP downloaded files #206

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f0643fb
update view to show contributors
pengyin-shan May 19, 2022
b74dd0c
add contributor to plan cover page if there is any
pengyin-shan May 19, 2022
8b8a056
add title for csv coverpage
pengyin-shan May 19, 2022
d999544
improve syntax
pengyin-shan May 20, 2022
48a3b50
update syntax
pengyin-shan May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

### Added

- Added contributors to plan's cover page (if there is any) [#202](https://github.com/portagenetwork/roadmap/issues/202)
- Added plan title to csv exported file

## [3.0.4+portage-3.0.12] - 2022-05-12

### Added
Expand Down
7 changes: 7 additions & 0 deletions app/controllers/plan_exports_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ def show
@selected_phase = @plan.phases.order("phases.updated_at DESC")
.detect { |p| p.visibility_allowed?(@plan) }
end

#ISSUE205: add contributor to cover page
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add explanation to the behaviour of the code instead of referencing the issue number of the repository. If we ever change repositories the issue number will no longer make sense.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add tests to make sure all the people listed actually show up on the reports.

Yeah, I was also considering this. I submitted the issue DMPRoadmap#3179 to DMPRoadmap because our current way of testing is not tracking the downloaded file, instead, it is just tracking the original page source.

So far I haven't figured out the best way of asking RSpec to track the content of the 'downloaded file' (i.e. docx, csv, pdf..) because it is part of the user's computer instead of the rails program. I will continue to think about it. Please let me know if you know anything I can refer it.

@hash[:data_curation] = Contributor.where(:plan_id => @plan.id).data_curation
@hash[:investigation] = Contributor.where(:plan_id => @plan.id).investigation
@hash[:pa] = Contributor.where(:plan_id => @plan.id).project_administration
@hash[:other] = Contributor.where(:plan_id => @plan.id).other
lagoan marked this conversation as resolved.
Show resolved Hide resolved

respond_to do |format|
format.html { show_html }
format.csv { show_csv }
Expand Down
20 changes: 19 additions & 1 deletion app/models/concerns/exportable_plan.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ def prepare_coversheet
attribution << role.user.name(false)
end
hash[:attribution] = attribution
#ISSUE205: add contributor to cover page
hash[:data_curation] = Contributor.where(:plan_id => id).data_curation
hash[:investigation] = Contributor.where(:plan_id => id).investigation
hash[:pa] = Contributor.where(:plan_id => id).project_administration
hash[:other] = Contributor.where(:plan_id => id).other
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comments above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


# Org name of plan owner's org
hash[:affiliation] = owner.present? ? owner.org.name : ""
Expand All @@ -127,12 +132,25 @@ def prepare_coversheet

# rubocop:disable Metrics/MethodLength, Metrics/AbcSize
def prepare_coversheet_for_csv(csv, _headings, hash)
csv << [_("Title: "), _("%{title}") % { title: title }]
csv << [if hash[:attribution].many?
_("Creators: ")
else
_("Creator:")
end, _("%{authors}") % { authors: hash[:attribution].join(", ") }]
csv << ["Affiliation: ", _("%{affiliation}") % { affiliation: hash[:affiliation] }]
if hash[:investigation].present?
csv << [_("Principal Investigator: "), _("%{investigation}") % { investigation: hash[:investigation].map(&:name).join(', ') }]
end
if hash[:data_curation].present?
csv << [_("Date Manager: "), _("%{data_curation}") % { data_curation: hash[:data_curation].map(&:name).join(', ') }]
end
if hash[:pa].present?
csv << [_("Project Administrator: "), _("%{pa}") % { pa: hash[:pa].map(&:name).join(', ') }]
end
if hash[:other].present?
csv << [_("Contributor: "), _("%{other}") % { other: hash[:other].map(&:name).join(', ') }]
end
csv << [_("Affiliation: "), _("%{affiliation}") % { affiliation: hash[:affiliation] }]
lagoan marked this conversation as resolved.
Show resolved Hide resolved
csv << if hash[:funder].present?
[_("Template: "), _("%{funder}") % { funder: hash[:funder] }]
else
Expand Down
15 changes: 15 additions & 0 deletions app/views/shared/export/_plan_coversheet.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@
<%# Allow raw html (==) for plan_attribution as it has <b> tags %>
<p><%== plan_attribution(@hash[:attribution]) %></p><br>

<%# ISSUE205: print out contributors if exit %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment about referencing repository issues.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

<%# Roles are ranked by PI -> DM -> PA -> Other (if any) %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good explanation of the behaviour of the view!

<% if @hash[:investigation].present? %>
<p><b><%= _("Principal Investigator: ") %></b><%= @hash[:investigation].map(&:name).join(', ') %></p><br>
<% end %>
<% if @hash[:data_curation].present? %>
<p><b><%= _("Data Manager: ") %></b><%= @hash[:data_curation].map(&:name).join(', ') %></p><br>
<% end %>
<% if @hash[:pa].present? %>
<p><b><%= _("Project Administrator: ") %></b><%= @hash[:pa].map(&:name).join(', ') %></p><br>
<% end %>
<% if @hash[:other].present? %>
<p><b><%= _("Contributor: ") %></b><%= @hash[:other].map(&:name).join(', ') %></p><br>
<% end %>

<p><b><%= _("Affiliation: ") %></b><%= @hash[:affiliation] %></p><br>

<% if @hash[:funder].present? %>
Expand Down
15 changes: 15 additions & 0 deletions app/views/shared/export/_plan_txt.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
<%= "#{@plan.title}" %>
<%= "----------------------------------------------------------\n" %>
<% if @show_coversheet %>

<%= @hash[:attribution].many? ? _("Creators: ") : _('Creator:') %> <%= @hash[:attribution].join(', ') %>
<%# ISSUE205: print out contributors if exit %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

<%# Roles are ranked by PI -> DM -> PA -> Other (if any) %>
<% if @hash[:investigation].present? %>
<%= _("Principal Investigator: ") + @hash[:investigation].map(&:name).join(', ') %>
<% end %>
<% if @hash[:data_curation].present? %>
<%= _("Data Manager: ") + @hash[:data_curation].map(&:name).join(', ') %>
<% end %>
<% if @hash[:pa].present? %>
<%= _("Project Administrator: ") + @hash[:pa].map(&:name).join(', ') %>
<% end %>
<% if @hash[:other].present? %>
<%= _("Contributor: ") + @hash[:other].map(&:name).join(', ') %>
<% end %>
<%= _("Affiliation: ") + @hash[:affiliation] %>
<% if @hash[:funder].present? %>
<%= _("Template: ") + @hash[:funder] %>
Expand Down