Skip to content

Commit

Permalink
Revert "peer_review: remove unnecessary peer_review_id foreign key th…
Browse files Browse the repository at this point in the history
…at was creating a circular reference"

This reverts commit d546080.
  • Loading branch information
mishaschwartz committed Dec 20, 2022
1 parent d546080 commit 94ef987
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 27 deletions.
7 changes: 3 additions & 4 deletions app/controllers/results_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion app/helpers/random_assign_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions app/models/peer_review.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions app/models/submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 0 additions & 14 deletions db/migrate/20221219204837_remove_peer_review_id_from_results.rb

This file was deleted.

19 changes: 17 additions & 2 deletions db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down Expand Up @@ -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: -
--
Expand Down Expand Up @@ -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: -
--
Expand Down Expand Up @@ -4683,5 +4699,4 @@ INSERT INTO "schema_migrations" (version) VALUES
('20220922131809'),
('20221019191315'),
('20221111182002'),
('20221114170146'),
('20221219204837');
('20221114170146');

0 comments on commit 94ef987

Please sign in to comment.