Changing affiliation from code to name (#1058)

* changing affiliation from code to name * ensures code is saved to database, not title * trying to add tests: * changing affiliation display * allowing all affiliations to be displayed * fixing failing tests * Rebasing against `main` and adjusting the Affiliation name for RDSS; Ensuring that the Controller tests pass where Views must be explicitly rendered --------- Co-authored-by: Lauren Malek <[email protected]> Co-authored-by: Bess Sadler <[email protected]>
pulibrary · Dec 2, 2024 · 095a6d0 · 095a6d0
1 parent c08ed20
commit 095a6d0
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 56 deletions.
diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js
@@ -3,3 +3,4 @@
 //= link_directory ../stylesheets .scss
 //= link_tree ../../javascript .js
 //= link_tree ../../../vendor/javascript .js
+//= link application.css
diff --git a/app/models/affiliation.rb b/app/models/affiliation.rb
@@ -4,7 +4,7 @@ def self.all
     data = []
     data << { code: "23100", name: "Astrophysical Sciences" }
     data << { code: "HPC", name: "High Performance Computing" }
-    data << { code: "RDSS", name: "Research Data and Scholarly Services" }
+    data << { code: "RDSS", name: "Research Data and Scholarship Services" }
     data << { code: "PRDS", name: "Princeton Research Data Service" }
     data << { code: "PPPL", name: "Princeton Plasma Physics Laboratory" }
     data

diff --git a/app/views/projects/_edit_form.html.erb b/app/views/projects/_edit_form.html.erb
@@ -2,7 +2,7 @@
 <div id="edit">
    <h2>Project Roles</h2>
 
-   <div class="row ">
+   <div class="row">
       <div class="col-md-3">
          <label for="data_sponsor">Data Sponsor</label>
          <div class="required-field">Required</div>
@@ -20,7 +20,7 @@
       </div>
    </div> <!-- row -->
 
-   <div class="row ">
+   <div class="row">
       <div class="col-md-3">
          <label for="data_manager">Data Manager</label>
          <div class="required-field">Required</div>
@@ -39,7 +39,7 @@
    </div> <!-- row -->
 
    <label>Data User(s)</label>
-   <div class="row ">
+   <div class="row">
       <div class="col-md-3">
          <label for="ro-user-uid-to-add">Read Only</label>
       </div>
@@ -60,9 +60,9 @@
             </div>
          </div>
       </div>
-   </div> <!-- row -->
+   </div><!-- row -->
 
-   <div class="row ">
+   <div class="row">
       <div class="col-md-3">
          <label for="rw-user-uid-to-add">Read/Write</label>
       </div>
@@ -87,7 +87,7 @@
 
    <h2>Project Description</h2>
       <% if current_user.superuser? %>
-         <div class="row ">
+         <div class="row">
             <div class="col-md-3">
                <label for="project_id">Project ID</label>
             </div>
@@ -96,7 +96,7 @@
             </div>
          </div> <!-- row -->
       <% end %>
-      <div class="row ">
+      <div class="row">
          <div class="col-md-3">
             <label for="title">Title</label>
             <div class="required-field">Required</div>
@@ -111,7 +111,7 @@
            Make the field readonly so the user cannot change it, but leave it as an HTML INPUT so that it is
            send back to the controller (we don't want to lose this value)
          -->
-         <div class="row ">
+         <div class="row">
            <div class="col-md-3">
              <label for="project_directory">Project Directory <%= @project.project_directory_parent_path %>/</label>
            </div>
@@ -121,7 +121,7 @@
          </div> <!-- row -->
 
          <% if (current_user.superuser? || current_user.eligible_sysadmin?) %>
-            <div class="row ">
+            <div class="row">
                <div class="col-md-3">
                   <label>MediaFlux ID</label>
                </div>
@@ -133,45 +133,44 @@
          <% end %>
       <% end %>
 
-      <div class="row ">
-         <div class="col-md-3">
-           <label for="project_directory">Directory Path</label>
-           <div class="required-field">Required</div>
-         </div>
-         <div class="col-md-9">
-           <span class="path-info"><%= @project.project_directory_parent_path %>/ </span>
-           <input type="text" aria-label="project directory" id="project_directory" name="project_directory" required oninvalid="this.setCustomValidity('')" oninput="this.setCustomValidity('')" value="<%= @project.project_directory_short %>" pattern="[\w\p{L}\-]{1,64}" />
-         </div>
-       </div>  <!-- row -->
+      <div class="row">
+        <div class="col-md-3">
+          <label for="project_directory">Directory Path</label>
+          <div class="required-field">Required</div>
+        </div>
+        <div class="col-md-9">
+          <span class="path-info"><%= @project.project_directory_parent_path %>/ </span>
+          <input type="text" aria-label="project directory" id="project_directory" name="project_directory" required oninvalid="this.setCustomValidity('')" oninput="this.setCustomValidity('')" value="<%= @project.project_directory_short %>" pattern="[\w\p{L}\-]{1,64}" />
+        </div>
+      </div><!-- row -->
 
-      <div class="row ">
-         <div class="col-md-3">
-            <label for="departments">Departments</label>
-            <div class="required-field">Required</div>
-         </div>
-         <div class="col-md-9">
-            <select id="departments" name="departments[]"  aria-label="project department" required value class="form-select" multiple >
-                           <% Affiliation.all.each do |affiliation| %>
-                           <% if @project.departments.include?(affiliation[:code]) %>
-                           <option selected><%= affiliation[:code] %></option>
-                           <% else %>
-                           <option><%= affiliation[:code] %></option>
-                           <% end %>
-                           <% end %>
-                        </select>
-            </div>
-         </div>
-      </div> <!-- row -->
+      <div class="row">
+        <div class="col-md-3">
+          <label for="departments">Departments</label>
+          <div class="required-field">Required</div>
+        </div>
+        <div class="col-md-9">
+          <select id="departments" name="departments[]"  aria-label="project department" required value class="form-select" multiple >
+            <% Affiliation.all.each do |affiliation| %>
+              <% if @project.departments.include?(affiliation[:code]) %>
+                <option value="<%= affiliation[:code] %>" selected><%= affiliation[:name] %></option>
+              <% else %>
+                <option value="<%= affiliation[:code] %>"><%= affiliation[:name] %></option>
+              <% end %>
+            <% end %>
+          </select>
+        </div>
+      </div><!-- row -->
 
-      <div class="row ">
+      <div class="row">
          <div class="col-md-3">
             <label for="description">Description</label>
          </div>
          <div class="col-md-9">
             <textarea type="text" id="description"  aria-label="project description" name="description" class="input-text-long"
                         rows="5" cols="72" placeholder=""><%= @project.metadata_model.description %></textarea>
          </div>
-      </div> <!-- row -->
+      </div><!-- row -->
 
       <!-- Keeps track of added read-only and read-write users -->
       <input type="text" id="ro_user_counter" name="ro_user_counter" value="0" style="display:none;"/>
@@ -185,8 +184,7 @@
          <%= link_to "Cancel", root_path, class: "btn btn-secondary", id: "cancel-btn" %>
          <% end %>
       </div>
-
-</div> <!-- id=edit -->
+</div><!-- id=edit -->
 
 <script>
    // Notice that we are keeping this JavaScript on the page rather than on application.js

diff --git a/app/views/projects/show.html.erb b/app/views/projects/show.html.erb
@@ -66,7 +66,15 @@ Project Details:
     <% if @departments.empty? %>
       <dd><strong class="px-0">None</strong></dd>
     <% else %>
-      <dd><%= @departments %></dd>
+      <dd>
+        <% @project.project.metadata_json["departments"].each do |department_code| %>
+          <% Affiliation.all.each do |hash| %>
+            <% if hash[:code] == department_code %>
+            <%= hash[:name] %>
+            <% end %>      
+          <% end %>
+        <% end %>
+      </dd>
     <% end %>
     <dt>Project Directory</dt>
     <dd><%= @project.project_directory %></dd>

diff --git a/spec/controllers/mediaflux_info_controller_spec.rb b/spec/controllers/mediaflux_info_controller_spec.rb
@@ -15,7 +15,7 @@
     expect(response.body).to eq(docker_response).or eq(ansible_response)
   end
 
-  it "does not retry infinately" do
+  it "does not retry infinitely" do
     original_pass = Rails.configuration.mediaflux["api_password"]
     original_session = user.mediaflux_session
 

diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require "rails_helper"
 
-RSpec.describe ProjectsController do
+RSpec.describe ProjectsController, type: ["controller", "feature"] do
   let(:project) { FactoryBot.create :project }
   describe "#show" do
     it "renders an error when requesting json" do
@@ -22,6 +22,15 @@
         expect(JSON.parse(response.body)).to eq(project.metadata)
       end
 
+      it "shows affiliation code as being saved to the project" do
+        get :show, params: { id: project.id, format: :json }
+        expect(JSON.parse(response.body)["departments"]).to include("23100")
+          .or(include("HPC"))
+          .or(include("RDSS"))
+          .or(include("PRDS"))
+          .or(include("PPPL"))
+      end
+
       it "renders the project metadata as xml" do
         project = FactoryBot.create :project, project_id: "abc-123"
         get :show, params: { id: project.id, format: :xml }
@@ -91,4 +100,54 @@
       end
     end
   end
+
+  context "when the project show views are rendered for an existing project" do
+    # Views are stubbed by default for rspec-rails
+    # https://rspec.info/features/6-0/rspec-rails/controller-specs/isolation-from-views/
+    render_views
+    let(:current_user) { FactoryBot.create(:user, uid: "pul123") }
+    let(:project) { FactoryBot.create(:project, data_sponsor: current_user.uid, data_manager: current_user.uid, title: "project 111") }
+
+    before do
+      project
+      sign_in(current_user)
+    end
+
+    it "shows the affiliation name (instead the internal code) on the project show views" do
+      get :show, params: { id: project.id }
+      expect(response).to render_template("show")
+      expect(response.body).to have_content("Astrophysical Sciences")
+        .or(have_content("High Performance Computing"))
+        .or(have_content("Research Data and Scholarship Services"))
+        .or(have_content("Princeton Research Data Service"))
+        .or(have_content("Princeton Plasma Physics Laboratory"))
+    end
+  end
+
+  context "when the project edit views are rendered for an existing project" do
+    # Views are stubbed by default for rspec-rails
+    # https://rspec.info/features/6-0/rspec-rails/controller-specs/isolation-from-views/
+    render_views
+    let(:current_user) { FactoryBot.create(:superuser, uid: "pul123") }
+    let(:approved_project) do
+      project = FactoryBot.create(:approved_project, data_sponsor: current_user.uid, data_manager: current_user.uid, title: "project 111")
+      project.mediaflux_id = nil
+      project
+    end
+
+    before do
+      approved_project
+      sign_in(current_user)
+    end
+
+    it "shows the affiliation name (instead the internal code) on the project edit views" do
+      get :edit, params: { id: approved_project.id }
+      expect(response).to render_template("edit")
+      expect(response.body).to have_content("Astrophysical Sciences")
+        .or(have_content("High Performance Computing"))
+        .or(have_content("Research Data and Scholarship Services"))
+        .or(have_content("Princeton Research Data Service"))
+        .or(have_content("Princeton Plasma Physics Laboratory"))
+    end
+  end
 end
diff --git a/spec/system/project_spec.rb b/spec/system/project_spec.rb
@@ -103,6 +103,13 @@
     end
 
     context "a project is active" do
+      before do
+        sign_in sponsor_user
+        project_in_mediaflux.metadata_json["status"] = Project::APPROVED_STATUS
+        project_in_mediaflux.save!
+        visit "/projects/#{project_in_mediaflux.id}/edit"
+      end
+
       it "redirects the user to the project details page if the user is not a sponsor or manager" do
         sign_in read_only
         # project_in_mediaflux.metadata_json["status"] = Project::APPROVED_STATUS
@@ -113,13 +120,6 @@
         expect(page).to have_content("Only data sponsors and data managers can revise this project.")
       end
 
-      before do
-        sign_in sponsor_user
-        project_in_mediaflux.metadata_json["status"] = Project::APPROVED_STATUS
-        project_in_mediaflux.save!
-        visit "/projects/#{project_in_mediaflux.id}/edit"
-      end
-
       it "preserves the readonly directory field" do
         click_on "Submit"
         project_in_mediaflux.reload
@@ -131,7 +131,9 @@
       end
 
       it "redirects the user to the revision request confirmation page upon submission" do
+        page.save_screenshot
         click_on "Submit"
+        page.save_screenshot
         project_in_mediaflux.reload
         expect(project_in_mediaflux.metadata[:project_directory]).to eq "project-123"
 
@@ -179,7 +181,7 @@
       fill_in_and_out "ro-user-uid-to-add", with: read_only.uid
       fill_in_and_out "rw-user-uid-to-add", with: read_write.uid
       # select a department
-      select "RDSS", from: "departments"
+      select "Research Data and Scholarship Services", from: "departments"
       fill_in "project_directory", with: "test_project"
       fill_in "title", with: "My test project"
       expect(page).to have_content("/td-test-001/")
@@ -320,7 +322,7 @@
         fill_in_and_out "data_manager", with: data_manager.uid
         fill_in_and_out "ro-user-uid-to-add", with: read_only.uid
         fill_in_and_out "rw-user-uid-to-add", with: read_write.uid
-        select "RDSS", from: "departments"
+        select "Research Data and Scholarship Services", from: "departments"
         fill_in "project_directory", with: FFaker::Name.name.tr(" ", "_")
         fill_in "title", with: "My test project"
         expect(page).to have_content("/td-test-001/")
@@ -367,11 +369,15 @@
         fill_in_and_out "data_manager", with: data_manager.uid
         fill_in_and_out "ro-user-uid-to-add", with: read_only.uid
         fill_in_and_out "rw-user-uid-to-add", with: read_write.uid
-        select "RDSS", from: "departments"
+        select "Research Data and Scholarship Services", from: "departments"
         fill_in "project_directory", with: FFaker::Name.name.tr(" ", "_")
         fill_in "title", with: "My test project"
         expect(page).to have_content("/td-test-001/")
-        expect(page.find_all("input:invalid").count).to eq(0)
+        invalid_fields = page.find_all("input:invalid").count
+        # There seemed to be some cases where this might fail when run with a headless Chrome browser
+        # Inserting a delay ensured that this did not occur at all
+        sleep(0.1)
+        expect(invalid_fields).to eq(0)
         click_on "Submit"
         # For some reason the above click on submit sometimes does not submit the form
         #  even though the inputs are all valid, so try it again...