Repository: openreview/openreview-py (language: python)

Issue:
Venue: AC assignment validation should check meta reviews are not posted
It seems the AC assignment pre-process function is checking the anon reviewers group and review invitations. 

We should parametrize it and use area chair anon groups and meta review invitations. 

Also we should check if review_name or metareview_name exists before checking if there is already a reply.

I fixed the code in the live site:

https://openreview.net/invitation/edit?id=ICML.cc/2023/Conference/Area_Chairs/-/Assignment

How to reproduce it:

- assign reviewer to a paper
- assign AC to a paper
- reviewer submit the review
- unassign AC from a paper

Error got: `“error: Can not remove assignment, the user ~Emma_Brunskill2 already posted a review.”`

The AC can see all the reviewer anon groups so the query got a group and that reviewer already posted a review



Discussion / hints:

Fix patch:
diff --git a/openreview/venue/invitation.py b/openreview/venue/invitation.py
index 07980698a..1b4260c04 100644
--- a/openreview/venue/invitation.py
+++ b/openreview/venue/invitation.py
@@ -1654,6 +1654,9 @@ def set_assignment_invitation(self, committee_id):
 
         with open(os.path.join(os.path.dirname(__file__), 'process/assignment_pre_process.py')) as pre:
             pre_content = pre.read()
+            if is_area_chair:
+                pre_content = pre_content.replace("REVIEW_NAME_STRING = 'review_name'", "REVIEW_NAME_STRING = 'meta_review_name'")
+                pre_content = pre_content.replace("REVIEWERS_ANON_NAME_STRING = 'reviewers_anon_name'", "REVIEWERS_ANON_NAME_STRING = 'area_chairs_anon_name'")
             with open(os.path.join(os.path.dirname(__file__), 'process/assignment_post_process.py')) as post:
                 post_content = post.read()
                 post_content = post_content.replace("PAPER_GROUP_ID = ''", "PAPER_GROUP_ID = '" + paper_group_id + "'")
diff --git a/openreview/venue/process/assignment_post_process.py b/openreview/venue/process/assignment_post_process.py
index 9b518a332..04a80ee52 100644
--- a/openreview/venue/process/assignment_post_process.py
+++ b/openreview/venue/process/assignment_post_process.py
@@ -10,8 +10,6 @@ def process_update(client, edge, invitation, existing_edge):
     venue_id = domain.id
     short_phrase = domain.content['subtitle']['value']
     program_chairs_id = domain.content['program_chairs_id']['value']
-    review_name = domain.content['review_name']['value']
-    reviewers_anon_name = domain.content['reviewers_anon_name']['value']    
 
     note=client.get_note(edge.head)
     group=client.get_group(PAPER_GROUP_ID.format(number=note.number))
diff --git a/openreview/venue/process/assignment_pre_process.py b/openreview/venue/process/assignment_pre_process.py
index 2d71131b8..3f167fdca 100644
--- a/openreview/venue/process/assignment_pre_process.py
+++ b/openreview/venue/process/assignment_pre_process.py
@@ -3,14 +3,20 @@ def process_update(client, edge, invitation, existing_edge):
     domain = client.get_group(edge.domain)
     venue_id = domain.id
     submission_name = domain.content['submission_name']['value']
-    review_name = domain.content['review_name']['value']
-    reviewers_anon_name = domain.content['reviewers_anon_name']['value']
+
+    REVIEW_NAME_STRING = 'review_name'
+    REVIEWERS_ANON_NAME_STRING = 'reviewers_anon_name'
+    review_name = domain.content.get(REVIEW_NAME_STRING, {}).get('value')
+    reviewers_anon_name = domain.content[REVIEWERS_ANON_NAME_STRING]['value']
     
     if edge.ddate:
         paper=client.get_note(edge.head)
 
         paper_group_id=f'{venue_id}/{submission_name}{paper.number}'
 
+        if not review_name:
+            return
+
         reviews=client.get_notes(invitation=f'{paper_group_id}/-/{review_name}')
 
         if not reviews:
@@ -24,5 +30,5 @@ def process_update(client, edge, invitation, existing_edge):
 
         for review in reviews:
             if review.signatures[0] == groups[0].id:
-                raise openreview.OpenReviewException(f'Can not remove assignment, the user {edge.tail} already posted a review.')
+                raise openreview.OpenReviewException(f'Can not remove assignment, the user {edge.tail} already posted a {review_name}')
 


Test patch:
diff --git a/tests/test_icml_conference.py b/tests/test_icml_conference.py
index 377e32932..26c676fe1 100644
--- a/tests/test_icml_conference.py
+++ b/tests/test_icml_conference.py
@@ -214,7 +214,6 @@ def test_create_conference(self, client, openreview_client, helpers, profile_man
         assert 'financial_aid' in submission_invitation.edit['note']['content']
         assert 'subject_areas' in submission_invitation.edit['note']['content']
 
-
     def test_add_pcs(self, client, openreview_client, helpers):
 
         pc_client=openreview.Client(username='pc@icml.cc', password='1234')
@@ -1506,7 +1505,7 @@ def test_assignment(self, client, openreview_client, helpers, request_page, sele
         sac_group = pc_client_v2.get_group('ICML.cc/2023/Conference/Submission1/Senior_Area_Chairs')
         assert ['~SAC_ICMLTwo1'] == sac_group.members
 
-    def test_reviewer_reassingment(self, client, openreview_client, helpers, selenium, request_page):
+    def test_reviewer_reassignment(self, client, openreview_client, helpers, selenium, request_page):
 
         pc_client = openreview.api.OpenReviewClient(username='pc@icml.cc', password='1234')
         ac_client = openreview.api.OpenReviewClient(username='ac1@icml.cc', password='1234')
@@ -1766,6 +1765,17 @@ def test_reviewer_reassingment(self, client, openreview_client, helpers, seleniu
         assert len(invite_edges) == 1
         assert invite_edges[0].label == 'Declined: I am too busy.'
 
+        #delete assignments before review stage and not get key error
+        pc_client_v2=openreview.api.OpenReviewClient(username='pc@icml.cc', password='1234')
+
+        assignment = pc_client_v2.get_edges(invitation='ICML.cc/2023/Conference/Reviewers/-/Assignment', head=submissions[10].id, tail='~Reviewer_ICMLThree1')[0]
+        assignment.ddate = openreview.tools.datetime_millis(datetime.datetime.utcnow())
+        pc_client_v2.post_edge(assignment)
+
+        assignment = pc_client_v2.get_edges(invitation='ICML.cc/2023/Conference/Area_Chairs/-/Assignment', head=submissions[10].id, tail='~AC_ICMLOne1')[0]
+        assignment.ddate = openreview.tools.datetime_millis(datetime.datetime.utcnow())
+        pc_client_v2.post_edge(assignment)
+
     def test_review_stage(self, openreview_client, helpers):
 
         pc_client=openreview.Client(username='pc@icml.cc', password='1234')
@@ -1791,7 +1801,7 @@ def test_review_stage(self, openreview_client, helpers):
 
         ac_client = openreview.api.OpenReviewClient(username='ac1@icml.cc', password='1234')
         submissions = ac_client.get_notes(invitation='ICML.cc/2023/Conference/-/Submission', sort='number:asc')
-        assert len(submissions) == 60
+        assert len(submissions) == 59
         assert ['ICML.cc/2023/Conference',
         'ICML.cc/2023/Conference/Submission2/Senior_Area_Chairs',
         'ICML.cc/2023/Conference/Submission2/Area_Chairs',
@@ -2525,7 +2535,7 @@ def test_review_stage(self, openreview_client, helpers):
         assert anon_group_id in reviews[0].readers
 
 
-    def test_delete_assignents(self, openreview_client):
+    def test_delete_assignments(self, openreview_client):
 
         ac_client = openreview.api.OpenReviewClient(username='ac2@icml.cc', password='1234')
 
@@ -2536,7 +2546,7 @@ def test_delete_assignents(self, openreview_client):
         assignment.ddate = openreview.tools.datetime_millis(datetime.datetime.utcnow())
         assignment.signatures = [anon_group_id]
         
-        with pytest.raises(openreview.OpenReviewException, match=r'Can not remove assignment, the user ~Reviewer_ICMLOne1 already posted a review.'):
+        with pytest.raises(openreview.OpenReviewException, match=r'Can not remove assignment, the user ~Reviewer_ICMLOne1 already posted a Official_Review.'):
             ac_client.post_edge(assignment)
 
         assignment = ac_client.get_edges(invitation='ICML.cc/2023/Conference/Reviewers/-/Assignment', head=submissions[0].id, tail='~Celeste_ICML1')[0]
@@ -2544,6 +2554,17 @@ def test_delete_assignents(self, openreview_client):
         assignment.signatures = [anon_group_id]
         ac_client.post_edge(assignment)
 
+        #delete AC assignment of paper with a review with no error
+        pc_client_v2=openreview.api.OpenReviewClient(username='pc@icml.cc', password='1234')
+
+        assignment = pc_client_v2.get_edges(invitation='ICML.cc/2023/Conference/Area_Chairs/-/Assignment', head=submissions[0].id, tail='~AC_ICMLTwo1')[0]
+        assignment.ddate = openreview.tools.datetime_millis(datetime.datetime.utcnow())
+        pc_client_v2.post_edge(assignment)
+
+        #re-add AC to paper 1
+        assignment = pc_client_v2.get_edges(invitation='ICML.cc/2023/Conference/Area_Chairs/-/Assignment', head=submissions[0].id, tail='~AC_ICMLTwo1', trash=True)[0]
+        assignment.ddate = None
+        pc_client_v2.post_edge(assignment)
 
     def test_comment_stage(self, openreview_client, helpers):
 
@@ -2991,6 +3012,33 @@ def test_meta_review_stage(self, openreview_client, helpers):
         assert openreview_client.get_invitation('ICML.cc/2023/Conference/Submission4/-/Meta_Review')
         assert openreview_client.get_invitation('ICML.cc/2023/Conference/Submission5/-/Meta_Review')
 
+        ac_client = openreview.api.OpenReviewClient(username='ac2@icml.cc', password='1234')
+        submissions = ac_client.get_notes(invitation='ICML.cc/2023/Conference/-/Submission', sort='number:asc')
+
+        anon_groups = ac_client.get_groups(prefix='ICML.cc/2023/Conference/Submission1/Area_Chair_', signatory='~AC_ICMLTwo1')
+        anon_group_id = anon_groups[0].id
+
+        meta_review_edit = ac_client.post_note_edit(
+            invitation='ICML.cc/2023/Conference/Submission1/-/Meta_Review',
+            signatures=[anon_group_id],
+            note=openreview.api.Note(
+                content={
+                    'metareview': { 'value': 'This is a good paper' },
+                    'recommendation': { 'value': 'Accept'}
+                }
+            )
+        )
+
+        helpers.await_queue(openreview_client)
+
+        #try to delete AC assignment of paper with a submitted metareview
+        pc_client_v2=openreview.api.OpenReviewClient(username='pc@icml.cc', password='1234')
+
+        assignment = pc_client_v2.get_edges(invitation='ICML.cc/2023/Conference/Area_Chairs/-/Assignment', head=submissions[0].id, tail='~AC_ICMLTwo1')[0]
+        assignment.ddate = openreview.tools.datetime_millis(datetime.datetime.utcnow())
+
+        with pytest.raises(openreview.OpenReviewException, match=r'Can not remove assignment, the user ~AC_ICMLTwo1 already posted a Meta_Review.'):
+            pc_client_v2.post_edge(assignment)
 
     def test_decision_stage(self, openreview_client, helpers):