From 6d09e9e6b98467477ccc1cb4cfc9805555c05527 Mon Sep 17 00:00:00 2001
From: fosterfarrell9 <28628554+fosterfarrell9@users.noreply.github.com>
Date: Wed, 21 Aug 2024 19:44:12 +0200
Subject: [PATCH] Fix lecture edit page if active term is not set (#683)

* Add missing case that Term.active is nil

* Add unit tests for lecture#stale?

* Make lecture#older_than? method private

* Add cypress test for lecture edit page

* Make better use of .then()

Co-authored-by: Splines <37160523+Splines@users.noreply.github.com>

* Add docstring

* Remove obsolete line

* Rename for consistency

* Add missing punctuation to docstring

---------

Co-authored-by: Splines <37160523+Splines@users.noreply.github.com>
Co-authored-by: Splines <dominic-plein@gmx.de>
---
 app/models/lecture.rb                  | 16 ++++++----
 app/views/lectures/edit/_form.html.erb |  1 +
 spec/cypress/e2e/lectures_spec.cy.js   | 15 +++++++++
 spec/factories/terms.rb                |  8 +++++
 spec/models/lecture_spec.rb            | 44 ++++++++++++++++++++++++++
 5 files changed, 78 insertions(+), 6 deletions(-)
 create mode 100644 spec/cypress/e2e/lectures_spec.cy.js

diff --git a/app/models/lecture.rb b/app/models/lecture.rb
index 7fd197471..973ac82d0 100644
--- a/app/models/lecture.rb
+++ b/app/models/lecture.rb
@@ -830,12 +830,9 @@ def speakers
     User.where(id: SpeakerTalkJoin.where(talk: talks).select(:speaker_id))
   end
 
-  def older_than?(timespan)
-    return true unless term
-
-    term.begin_date <= Term.active.begin_date - timespan
-  end
-
+  # Determines if the lecture is stale (i.e. older than one year).
+  # The age of the lecture is determined by the begin date of the term
+  # in which it was given and the begin date of the current term.
   def stale?
     older_than?(1.year)
   end
@@ -947,4 +944,11 @@ def only_one_lecture
 
       errors.add(:course, :already_present)
     end
+
+    def older_than?(timespan)
+      return false unless Term.active
+      return true unless term
+
+      term.begin_date <= Term.active.begin_date - timespan
+    end
 end
diff --git a/app/views/lectures/edit/_form.html.erb b/app/views/lectures/edit/_form.html.erb
index 5060e5b82..00e2608c3 100644
--- a/app/views/lectures/edit/_form.html.erb
+++ b/app/views/lectures/edit/_form.html.erb
@@ -15,6 +15,7 @@
             id="lecture-nav-content" type="button" role="tab"
              href="#content"
             data-bs-toggle="pill" data-bs-target="#lecture-pane-content"
+            data-cy="content-tab-btn"
             aria-controls="lecture-pane-content" aria-selected="true">
           <%= t('content') %>
         </button>
diff --git a/spec/cypress/e2e/lectures_spec.cy.js b/spec/cypress/e2e/lectures_spec.cy.js
new file mode 100644
index 000000000..96d3994ee
--- /dev/null
+++ b/spec/cypress/e2e/lectures_spec.cy.js
@@ -0,0 +1,15 @@
+import FactoryBot from "../support/factorybot";
+
+describe("Lecture edit page", () => {
+  it("shows content tab button", function () {
+    cy.createUserAndLogin("teacher").then((teacher) => {
+      FactoryBot.create("lecture", "with_teacher_by_id",
+        { teacher_id: teacher.id }).as("lecture");
+    });
+
+    cy.then(() => {
+      cy.visit(`/lectures/${this.lecture.id}/edit`);
+      cy.getBySelector("content-tab-btn").should("be.visible");
+    });
+  });
+});
diff --git a/spec/factories/terms.rb b/spec/factories/terms.rb
index 9a1c6e949..3ecd347c3 100644
--- a/spec/factories/terms.rb
+++ b/spec/factories/terms.rb
@@ -8,5 +8,13 @@
     trait :summer do
       season { "SS" }
     end
+
+    trait :winter do
+      season { "WS" }
+    end
+
+    trait :active do
+      active { true }
+    end
   end
 end
diff --git a/spec/models/lecture_spec.rb b/spec/models/lecture_spec.rb
index d2ba05682..59966f827 100644
--- a/spec/models/lecture_spec.rb
+++ b/spec/models/lecture_spec.rb
@@ -86,6 +86,50 @@
     end
   end
 
+  describe "#stale?" do
+    context "when there is no active term" do
+      it "returns false" do
+        lecture = FactoryBot.build(:lecture)
+        expect(lecture.stale?).to be(false)
+      end
+    end
+
+    context "when there is an active term" do
+      let(:year) { 2024 }
+
+      before(:each) do
+        FactoryBot.create(:term, :summer, :active, year: year)
+      end
+
+      context "and there is no term associated with the lecture" do
+        it "returns true" do
+          lecture = FactoryBot.build(:lecture, :term_independent)
+          expect(lecture.stale?).to be(true)
+        end
+      end
+
+      context "and the lecture term begin date is before the active term" \
+              "begin date minus 1 year" do
+        let(:lecture_term) { FactoryBot.build(:term, :summer, year: year - 1) }
+
+        it "returns true" do
+          lecture = FactoryBot.build(:lecture, term: lecture_term)
+          expect(lecture.stale?).to be(true)
+        end
+      end
+
+      context "when the lecture term begin date is not older than the" \
+              "active term begin date minus 1 year" do
+        let(:lecture_term) { FactoryBot.build(:term, :winter, year: year - 1) }
+
+        it "returns false" do
+          lecture = FactoryBot.build(:lecture, term: lecture_term)
+          expect(lecture.stale?).to be(false)
+        end
+      end
+    end
+  end
+
   # Test methods -- NEEDS TO BE REFACTORED
 
   # describe '#tags' do