peer_review: remove unnecessary peer_review_id foreign key that was c…

…reating a circular reference

app/controllers/results_controller.rb

@@ -142,7 +142,7 @@ def show
           reviewer_group = current_role.grouping_for(assignment.pr_assignment.id)
           data[:num_marked] = PeerReview.get_num_marked(reviewer_group)
           data[:num_collected] = PeerReview.get_num_collected(reviewer_group)
-          data[:group_name] = "#{PeerReview.model_name.human} #{result.peer_review_id}"
+          data[:group_name] = "#{PeerReview.model_name.human} #{result.peer_reviews.ids.first}"
           data[:members] = []
         end
 
@@ -315,10 +315,11 @@ def next_grouping
 
     if result.is_a_review? && current_role.is_reviewer_for?(assignment.pr_assignment, result)
       assigned_prs = current_role.grouping_for(assignment.pr_assignment.id).peer_reviews_to_others
+      peer_review_ids = result.peer_reviews.order(id: :asc).ids
       if params[:direction] == '1'
-        next_grouping = assigned_prs.where('peer_reviews.id > ?', result.peer_review_id).first
+        next_grouping = assigned_prs.where('peer_reviews.id > ?', peer_review_ids.last).first
       else
-        next_grouping = assigned_prs.where('peer_reviews.id < ?', result.peer_review_id).last
+        next_grouping = assigned_prs.where('peer_reviews.id < ?', peer_review_ids.first).last
       end
       next_result = Result.find_by(id: next_grouping&.result_id)
     else

app/helpers/random_assign_helper.rb

@@ -190,17 +190,10 @@ def save_peer_reviews(pr_assignment)
       )
     end
 
-    # Peer reviews require IDs to have been made, so they come last.
-    pr_ids = PeerReview.insert_all(
+    PeerReview.insert_all(
       results.zip(@reviewers).map do |result, reviewer_id|
         { reviewer_id: reviewer_id, result_id: result.id, created_at: now, updated_at: now }
       end
     )
-
-    Result.upsert_all(
-      results.zip(pr_ids).map do |result, pr_id|
-        { id: result.id, peer_review_id: pr_id['id'], view_token: result.view_token }
-      end
-    )
   end
 end

app/models/peer_review.rb

@@ -38,7 +38,6 @@ def self.create_peer_review_between(reviewer, reviewee)
       result = Result.create!(submission: reviewee.current_submission_used,
                               marking_state: Result::MARKING_STATES[:incomplete])
       peer_review = PeerReview.create!(reviewer: reviewer, result: result)
-      result.peer_review_id = peer_review.id
       result.save!
       peer_review
     end

app/models/result.rb

@@ -146,7 +146,7 @@ def mark_as_partial
   end
 
   def is_a_review?
-    !peer_review_id.nil?
+    peer_reviews.exists?
   end
 
   def is_review_for?(user, assignment)

app/models/submission.rb

@@ -15,11 +15,14 @@ class Submission < ApplicationRecord
            dependent: :destroy,
            inverse_of: :submission
 
-  has_many :non_pr_results, -> { where(peer_review_id: nil).order(:created_at) },
+  has_many :non_pr_results, -> { left_outer_joins(:peer_reviews).where('peer_reviews.id': nil).order(:created_at) },
            class_name: 'Result',
            inverse_of: :submission
 
-  has_one :current_result, -> { where(peer_review_id: nil).order(created_at: :desc) },
+  has_one :current_result, -> {
+                             left_outer_joins(:peer_reviews).where('peer_reviews.id': nil)
+                                                            .order(created_at: :desc)
+                           },
           class_name: 'Result',
           inverse_of: :submission
 

db/migrate/20221219204837_remove_peer_review_id_from_results.rb

@@ -0,0 +1,14 @@
+class RemovePeerReviewIdFromResults < ActiveRecord::Migration[7.0]
+  def up
+    remove_column :results, :peer_review_id
+  end
+  def down
+    add_reference :results, :peer_review, index: true
+    add_foreign_key :results, :peer_reviews, on_delete: :cascade
+    puts '-- Assigning peer review ids to results'
+    Result.includes(:peer_reviews).find_each do |result|
+      peer_review_id = result.peer_reviews.first&.id
+      result.update!(peer_review_id: peer_review_id) unless peer_review_id.nil?
+    end
+  end
+end

db/structure.sql

@@ -1574,7 +1574,6 @@ CREATE TABLE public.results (
     updated_at timestamp without time zone,
     released_to_students boolean DEFAULT false NOT NULL,
     remark_request_submitted_at timestamp without time zone,
-    peer_review_id integer,
     view_token character varying NOT NULL,
     view_token_expiry timestamp without time zone
 );
@@ -3600,13 +3599,6 @@ CREATE INDEX index_peer_reviews_on_reviewer_id ON public.peer_reviews USING btre
 CREATE INDEX index_periods_on_submission_rule_id ON public.periods USING btree (submission_rule_id);
 
 
---
--- Name: index_results_on_peer_review_id; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE INDEX index_results_on_peer_review_id ON public.results USING btree (peer_review_id);
-
-
 --
 -- Name: index_results_on_view_token; Type: INDEX; Schema: public; Owner: -
 --
@@ -4139,14 +4131,6 @@ ALTER TABLE ONLY public.levels
     ADD CONSTRAINT fk_rails_7d6e4d7d84 FOREIGN KEY (criterion_id) REFERENCES public.criteria(id);
 
 
---
--- Name: results fk_rails_81dcc9eed0; Type: FK CONSTRAINT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public.results
-    ADD CONSTRAINT fk_rails_81dcc9eed0 FOREIGN KEY (peer_review_id) REFERENCES public.peer_reviews(id) ON DELETE CASCADE;
-
-
 --
 -- Name: test_group_results fk_rails_848004f82a; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -4699,4 +4683,5 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20220922131809'),
 ('20221019191315'),
 ('20221111182002'),
-('20221114170146');
+('20221114170146'),
+('20221219204837');