diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb index ee9de2bb9a9..61a45e6e6e4 100644 --- a/app/controllers/results_controller.rb +++ b/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_reviews.ids.first}" + data[:group_name] = "#{PeerReview.model_name.human} #{result.peer_review_id}" data[:members] = [] end @@ -315,11 +315,10 @@ 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 > ?', peer_review_ids.last).first + next_grouping = assigned_prs.where('peer_reviews.id > ?', result.peer_review_id).first else - next_grouping = assigned_prs.where('peer_reviews.id < ?', peer_review_ids.first).last + next_grouping = assigned_prs.where('peer_reviews.id < ?', result.peer_review_id).last end next_result = Result.find_by(id: next_grouping&.result_id) else diff --git a/app/helpers/random_assign_helper.rb b/app/helpers/random_assign_helper.rb index e99eecd75ca..b5612f152ca 100644 --- a/app/helpers/random_assign_helper.rb +++ b/app/helpers/random_assign_helper.rb @@ -190,10 +190,17 @@ def save_peer_reviews(pr_assignment) ) end - PeerReview.insert_all( + # Peer reviews require IDs to have been made, so they come last. + pr_ids = 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 diff --git a/app/models/peer_review.rb b/app/models/peer_review.rb index d72a156ac47..85467d380ce 100644 --- a/app/models/peer_review.rb +++ b/app/models/peer_review.rb @@ -38,6 +38,7 @@ 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 diff --git a/app/models/result.rb b/app/models/result.rb index 7e2ed23d9ef..3c607f7fea4 100644 --- a/app/models/result.rb +++ b/app/models/result.rb @@ -146,7 +146,7 @@ def mark_as_partial end def is_a_review? - peer_reviews.exists? + !peer_review_id.nil? end def is_review_for?(user, assignment) diff --git a/app/models/submission.rb b/app/models/submission.rb index aa76179b5cb..ff3094de93f 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -15,14 +15,11 @@ class Submission < ApplicationRecord dependent: :destroy, inverse_of: :submission - has_many :non_pr_results, -> { left_outer_joins(:peer_reviews).where('peer_reviews.id': nil).order(:created_at) }, + has_many :non_pr_results, -> { where(peer_review_id: nil).order(:created_at) }, class_name: 'Result', inverse_of: :submission - has_one :current_result, -> { - left_outer_joins(:peer_reviews).where('peer_reviews.id': nil) - .order(created_at: :desc) - }, + has_one :current_result, -> { where(peer_review_id: nil).order(created_at: :desc) }, class_name: 'Result', inverse_of: :submission diff --git a/db/migrate/20221219204837_remove_peer_review_id_from_results.rb b/db/migrate/20221219204837_remove_peer_review_id_from_results.rb deleted file mode 100644 index bf71d7072f7..00000000000 --- a/db/migrate/20221219204837_remove_peer_review_id_from_results.rb +++ /dev/null @@ -1,14 +0,0 @@ -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 diff --git a/db/structure.sql b/db/structure.sql index bdcbc6f44bc..6e4ce12b921 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1574,6 +1574,7 @@ 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 ); @@ -3599,6 +3600,13 @@ 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: - -- @@ -4131,6 +4139,14 @@ 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: - -- @@ -4683,5 +4699,4 @@ INSERT INTO "schema_migrations" (version) VALUES ('20220922131809'), ('20221019191315'), ('20221111182002'), -('20221114170146'), -('20221219204837'); +('20221114170146');