From a6ba1759a39bbf4332cb3b6e86fd376d413913a3 Mon Sep 17 00:00:00 2001
From: Tuan Nguyen <amtuannguyen@gmail.com>
Date: Mon, 21 Oct 2024 11:19:19 -0400
Subject: [PATCH 1/2] improve system tests and modal dialogs

---
 .github/workflows/rubyonrails.yml             | 28 ++++++++++----
 .../_committee_member.html.erb                |  8 ++--
 app/views/committee_members/_modal.html.erb   |  2 +-
 app/views/committee_members/create.js.erb     |  2 +-
 app/views/committee_members/new.js.erb        |  2 +-
 ..._upload_modal.html.erb => _modal.html.erb} |  2 +-
 app/views/documents/edit.js.erb               |  7 ++--
 app/views/documents/new.js.erb                |  7 ++--
 test/application_system_test_case.rb          |  2 +-
 test/system/students_test.rb                  | 12 +++---
 test/system/theses_test.rb                    | 38 +++++++++++++++----
 11 files changed, 73 insertions(+), 37 deletions(-)
 rename app/views/documents/{_file_upload_modal.html.erb => _modal.html.erb} (77%)

diff --git a/.github/workflows/rubyonrails.yml b/.github/workflows/rubyonrails.yml
index cb1ba38..3318434 100644
--- a/.github/workflows/rubyonrails.yml
+++ b/.github/workflows/rubyonrails.yml
@@ -34,16 +34,28 @@ jobs:
           RAILS_ENV: test
           DATABASE_URL: sqlite3:db/test.sqlite3
         run: |
-          bundle exec rake db:reset
-          bundle exec rake db:seed
-          bundle exec rake test TESTOPTS="-v"
-          bundle exec rake test:system TESTOPTS="-v"
+          bundle exec rails db:reset
+          bundle exec rails db:seed
+          bundle exec rails test -v
+          bundle exec rails test:system TESTOPTS="-v"
+
       - name: Run tests with mysql
         env:
           RAILS_ENV: test
           DATABASE_URL: mysql2://test:test@127.0.0.1:3306/test
         run: |
-          bundle exec rake db:reset
-          bundle exec rake db:seed
-          bundle exec rake test TESTOPTS="-v"
-          bundle exec rake test:system TESTOPTS="-v"
+          bundle exec rails db:reset
+          bundle exec rails db:seed
+          bundle exec rails test -v
+          bundle exec rails test:system TESTOPTS="-v"
+
+      - name: Create tar archive of screenshots
+        run: |
+          ( cd /home/runner/work/etd/etd/tmp && [ -d ./test-screenshots ] && tar cvf test-screenshots.tar ./test-screenshots ) || true
+
+      - name: Upload screenshots
+        uses: actions/upload-artifact@v3
+        if: always()
+        with:
+          name: test-screenshots
+          path: /home/runner/work/etd/etd/tmp/test-screenshots.tar
diff --git a/app/views/committee_members/_committee_member.html.erb b/app/views/committee_members/_committee_member.html.erb
index 3567ff8..1ff5a56 100644
--- a/app/views/committee_members/_committee_member.html.erb
+++ b/app/views/committee_members/_committee_member.html.erb
@@ -1,8 +1,10 @@
 <div class="committee-member" id="committee_member_<%= committee_member.id %>">
-  <label><span class="committee-member-name fw-normal"><%= committee_member.name %></span> <span class="fw-normal fst-italic">(<%= committee_member.role %>)</span></label>
-  <%= hidden_field_tag "committee_member_ids[]", committee_member.id %>
+  <span class="committee-member-name fw-normal"><%= committee_member.name %></span> <span class="fw-normal fst-italic">(<%= committee_member.role %>)</span>
   <% if @thesis.id && can?(:destroy, committee_member) && @thesis.status != Thesis::PUBLISHED  %>
       <%= link_to student_thesis_committee_member_path(@student, @thesis, committee_member),
-        method: :delete, class: "btn btn-close", remote: true, "data-delete": "#{committee_member.name} (#{committee_member.role})" do %>&nbsp;<% end %>  
+        method: :delete, remote: true, class: "btn btn-close" do %>
+        <span class="sr-only">Remove <%= "#{committee_member.name} (#{committee_member.role})" %></span>
+        <% end %>  
   <% end %>
+  <%= hidden_field_tag "committee_member_ids[]", committee_member.id %>
 </div>
diff --git a/app/views/committee_members/_modal.html.erb b/app/views/committee_members/_modal.html.erb
index 72be84f..c1a043a 100644
--- a/app/views/committee_members/_modal.html.erb
+++ b/app/views/committee_members/_modal.html.erb
@@ -1,4 +1,4 @@
-<div class="modal fade" id="add_committee_member_modal" data-bs-keyboard="false" tabindex="-1" aria-hidden="true" data-bs-backdrop="static">
+<div class="modal" id="add_committee_member_modal" tabindex="-1" aria-hidden="true">
   <div class="modal-dialog modal-dialog-centered modal-dialog-scrollable">
     <div class="modal-content">
       <div class="modal-header">
diff --git a/app/views/committee_members/create.js.erb b/app/views/committee_members/create.js.erb
index 86d7a94..a96ee22 100644
--- a/app/views/committee_members/create.js.erb
+++ b/app/views/committee_members/create.js.erb
@@ -1,7 +1,7 @@
 <% if @committee_member.errors.blank? %>	
-	$("#committee_members .committee_members_list").append("<%= j render @committee_member %>");
 	$("#add_committee_member_modal").modal('hide');
 	$("#add_committee_member_modal").remove();
+	$("#committee_members .committee_members_list").append("<%= j render @committee_member %>");
 <% else %>
 	$("#add_committee_member_modal .modal-body").html('<%= j render("form") %>');
 <% end %>
\ No newline at end of file
diff --git a/app/views/committee_members/new.js.erb b/app/views/committee_members/new.js.erb
index f933a5f..967b8d1 100644
--- a/app/views/committee_members/new.js.erb
+++ b/app/views/committee_members/new.js.erb
@@ -1,2 +1,2 @@
-$("body").append('<%= j render("modal") %>');
+$('body').append('<%= j render("modal") %>');
 (new bootstrap.Modal($('#add_committee_member_modal'))).show();
diff --git a/app/views/documents/_file_upload_modal.html.erb b/app/views/documents/_modal.html.erb
similarity index 77%
rename from app/views/documents/_file_upload_modal.html.erb
rename to app/views/documents/_modal.html.erb
index ecb8f19..aa96eef 100644
--- a/app/views/documents/_file_upload_modal.html.erb
+++ b/app/views/documents/_modal.html.erb
@@ -1,4 +1,4 @@
-<div class="modal fade" id="file_upload_modal" data-bs-keyboard="false" tabindex="-1" aria-hidden="true" data-bs-backdrop="static">
+<div class="modal" id="file_upload_modal" tabindex="-1" aria-hidden="true">
   <div class="modal-dialog modal-dialog-centered modal-dialog-scrollable">
     <div class="modal-content">
       <div class="modal-header">
diff --git a/app/views/documents/edit.js.erb b/app/views/documents/edit.js.erb
index 97b1239..550ac9a 100644
--- a/app/views/documents/edit.js.erb
+++ b/app/views/documents/edit.js.erb
@@ -1,7 +1,6 @@
-$('#file_upload_modal_container').remove();
-$('body').append('<div id="file_upload_modal_container"></div>');
-$('#file_upload_modal_container').append('<%= j render "file_upload_modal" %>');
-new bootstrap.Modal(document.getElementById('file_upload_modal')).show();
+$('body').append('<%= j render "modal" %>');
+(new bootstrap.Modal($('#file_upload_modal'))).show();
+
 $('form.file-upload input.file').change ( function() {
     if ($(this).val()) {
         $(this).parents('form:first').find('input:submit').removeAttr('disabled');
diff --git a/app/views/documents/new.js.erb b/app/views/documents/new.js.erb
index 97b1239..550ac9a 100644
--- a/app/views/documents/new.js.erb
+++ b/app/views/documents/new.js.erb
@@ -1,7 +1,6 @@
-$('#file_upload_modal_container').remove();
-$('body').append('<div id="file_upload_modal_container"></div>');
-$('#file_upload_modal_container').append('<%= j render "file_upload_modal" %>');
-new bootstrap.Modal(document.getElementById('file_upload_modal')).show();
+$('body').append('<%= j render "modal" %>');
+(new bootstrap.Modal($('#file_upload_modal'))).show();
+
 $('form.file-upload input.file').change ( function() {
     if ($(this).val()) {
         $(this).parents('form:first').find('input:submit').removeAttr('disabled');
diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb
index f52b520..64fe93c 100644
--- a/test/application_system_test_case.rb
+++ b/test/application_system_test_case.rb
@@ -25,7 +25,7 @@ def setup
 
     super
 
-    current_window.resize_to(1280, 800)
+    current_window.resize_to(1920, 1080)
     
     user = FactoryGirl.create(:user)
     login_as(user, role: User::STAFF)
diff --git a/test/system/students_test.rb b/test/system/students_test.rb
index 4443e4e..9553641 100644
--- a/test/system/students_test.rb
+++ b/test/system/students_test.rb
@@ -20,6 +20,8 @@ class StudentsTest < ApplicationSystemTestCase
     click_link('Create ETD Student Record')
     page.accept_alert
 
+    sleep 5
+
     click_link('Send invitation email')
 
     assert_selector '.alert-success', text: "Sent an invitation email to #{@gem_record.studentname}"
@@ -225,9 +227,6 @@ class StudentsTest < ApplicationSystemTestCase
     login_as(@thesis.student)
     visit root_url
 
-    # Set Page size
-    page.driver.browser.manage.window.resize_to(1920, 2500)
-
     ## Page 1
     click_link("My ETD Submission")
     assert_selector "input#student_email_external"
@@ -315,6 +314,10 @@ class StudentsTest < ApplicationSystemTestCase
     assert page.has_selector?('#thesis_etd_licence_agreement', visible: true), "#thesis_etd_licence_agreement not found."
     checkbox = find('#thesis_etd_licence_agreement', visible: true)
     checkbox.check
+
+    # wait a little, this is necessary when running on Github actions
+    sleep 2
+
     assert checkbox.checked?, "#thesis_etd_licence_agreement checkbox is not checked."
 
     # Ensure you can't go next without uploading LAC Document
@@ -343,9 +346,6 @@ class StudentsTest < ApplicationSystemTestCase
     login_as(@thesis.student)
     visit root_url
 
-    # Set Page size
-    page.driver.browser.manage.window.resize_to(1920, 2500)
-
     ## Page 1
     click_link("My ETD Submission")
     assert_selector "input#student_email_external"
diff --git a/test/system/theses_test.rb b/test/system/theses_test.rb
index 62ba7f2..5bc433e 100644
--- a/test/system/theses_test.rb
+++ b/test/system/theses_test.rb
@@ -70,19 +70,43 @@ class ThesesTest < ApplicationSystemTestCase
     visit root_url
     click_link(@thesis_01.title)
     click_on('Make Changes')
+    
     click_on('Add committee member')
-
-    fill_in('First Name', with: 'test1')
-    fill_in('Last Name', with: 'test2')
+    fill_in('First Name', with: 'f1')
+    fill_in('Last Name', with: 'l1')
     choose('Committee Member')
     click_on('Add')
-    assert_selector 'span', text: 'test2, test1'
 
-    assert_selector 'a[data-delete="test2, test1 (Committee Member)"]', visible: true
+    # wait for AJAX request to complete, this is necessary when running on Github actions
+    sleep 5
+
+    assert_selector 'span', text: 'l1, f1'
+
+    click_on('Add committee member')
+    fill_in('First Name', with: 'f2')
+    fill_in('Last Name', with: 'l2')
+    choose('Chair')
+    click_on('Add')
+
+    # wait for AJAX request to complete, this is necessary when running on Github actions
+    sleep 5
+
+    assert_selector 'span', text: 'l2, f2'
+
+    remove1 = 'Remove l1, f1 (Committee Member)'
+    remove2 = 'Remove l2, f2 (Chair)'
+
+    assert has_link?(remove1)
+    assert has_link?(remove2)
+    
+    click_link_or_button(remove1)
     
-    find('a[data-delete="test2, test1 (Committee Member)"]').click
+    assert_no_selector 'span', text: 'l1, f1'
+    assert_no_link(remove1)
+
+    assert_selector 'span', text: 'l2, f2'
 
-    assert_no_selector 'span', text: 'test2, test1'
+    assert has_link?(remove2)
   end
 
   test 'Edit a thesis' do

From 2a8b39e52a7e54fa0e55714f355ff274504b60e1 Mon Sep 17 00:00:00 2001
From: Tuan Nguyen <amtuannguyen@gmail.com>
Date: Mon, 21 Oct 2024 11:47:02 -0400
Subject: [PATCH 2/2] increase wait time to 10s

---
 test/system/students_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/system/students_test.rb b/test/system/students_test.rb
index 9553641..b1b367d 100644
--- a/test/system/students_test.rb
+++ b/test/system/students_test.rb
@@ -20,7 +20,7 @@ class StudentsTest < ApplicationSystemTestCase
     click_link('Create ETD Student Record')
     page.accept_alert
 
-    sleep 5
+    sleep 10
 
     click_link('Send invitation email')