Skip to content

Commit

Permalink
Update request_remove_name_process.py (#2419)
Browse files Browse the repository at this point in the history
* Update request_remove_name_process.py

Fix the reversed names that weren't being approved.

* Account for profile ID

The original PR wasn't accounting for the fact that when multiple accounts use the same name, numbers are used to differentiate between preferred names, so most names were not automatically passing. The tests were passing because there were no publications associated with the name, which also triggers automatic acceptance.

This PR updates the tests and the process function to account for that fact

* Change preferred name to pretty=True

Makes it cleaner so we don't need to pre-process the preferred name as much. Also updated test_remove_name_from_merged_profile so it doesn't fail anymore

---------

Co-authored-by: celestemartinez <[email protected]>
  • Loading branch information
emily-grabowski and celestemartinez authored Nov 13, 2024
1 parent c04a678 commit 2b03531
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 36 deletions.
11 changes: 6 additions & 5 deletions openreview/profile/process/request_remove_name_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@ def process(client, edit, invitation):
usernames = edit.note.content['usernames']['value']

print("Check the name to be deleted against the preferred name for simple string operations")
preferred_name = client.get_profile(edit.note.signatures[0]).get_preferred_name()
name_to_delete = edit.note.content['name']['value']
preferred_name = client.get_profile(edit.note.signatures[0]).get_preferred_name(pretty=True)
name_to_delete =edit.note.content['name']['value']

proc_preferred_name = preferred_name.strip().lower().replace(' ','')
proc_rev_preferred_name = ''.join(preferred_name.split(' ')[::-1]).strip().lower().replace(' ','')
proc_name_to_delete = name_to_delete.strip().lower().replace(' ','')



if (proc_name_to_delete== proc_preferred_name) or (proc_name_to_delete== proc_rev_preferred_name):
print('Accepting the name removal request')
print('Automatic accept (similar name)')
client.post_note_edit(
invitation=REMOVAL_DECISION_INVITATION_ID,
signatures=[SUPPORT_USER_ID],
Expand Down Expand Up @@ -59,7 +60,7 @@ def process(client, edit, invitation):
signature=edit.domain)
return

print('Accepting the name removal request')
print('Automatic accept (no publications)')
client.post_note_edit(
invitation=REMOVAL_DECISION_INVITATION_ID,
signatures=[SUPPORT_USER_ID],
Expand Down
117 changes: 86 additions & 31 deletions tests/test_profile_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,8 @@ def test_remove_alternate_name(self, openreview_client, test_client, helpers):
'end': None
})
john_client.post_profile(profile)


profile = john_client.get_profile(email_or_id='~John_Last1')
assert len(profile.content['names']) == 2
assert 'username' in profile.content['names'][1]
Expand Down Expand Up @@ -604,18 +606,56 @@ def test_remove_alternate_name(self, openreview_client, test_client, helpers):
'''


##Make another profile with the same name:
john_two_client = helpers.create_user('[email protected]', 'John', 'Last', alternates=[], institution='google.com')

profile = john_two_client.get_profile()

profile.content['homepage'] = 'https://john.google.com'
profile.content['names'].append({
'first': 'John',
'middle': 'Alternate',
'last': 'Last'
})
profile.content['relations'].append({
'relation': 'Advisor',
'name': 'SomeFirstName User',
'username': '~SomeFirstName_User1',
'start': 2015,
'end': None
})
john_two_client.post_profile(profile)



#Try to automatically remove a name with different spacing/capitalization
profile=john_client.get_profile()
profile=john_two_client.get_profile()
profile.content['names'].append(
{'fullname':'johnlast'}
)
john_client.post_profile(profile)
profile=john_client.get_profile('~John_Last1')
john_two_client.post_profile(profile)

request_note = john_client.post_note_edit(

## add a publication
john_two_client.post_note_edit(
invitation='openreview.net/Archive/-/Direct_Upload',
signatures=['~johnlast1'],
note = openreview.api.Note(
pdate = openreview.tools.datetime_millis(datetime.datetime(2019, 4, 30)),
content = {
'title': { 'value': 'Paper title 2' },
'abstract': { 'value': 'Paper abstract 2' },
'authors': { 'value': ['John Last'] },
'authorids': { 'value': ['~johnlast1'] },
'venue': { 'value': 'Arxiv' }
},
license = 'CC BY-SA 4.0'
))
profile=john_two_client.get_profile('~John_Last2')

request_note = john_two_client.post_note_edit(
invitation='openreview.net/Support/-/Profile_Name_Removal',
signatures=['~John_Last1'],
signatures=['~John_Last2'],
note = openreview.api.Note(
content={
'name': { 'value': 'johnlast' },
Expand All @@ -626,21 +666,37 @@ def test_remove_alternate_name(self, openreview_client, test_client, helpers):
)
helpers.await_queue_edit(openreview_client, edit_id=request_note['id'])

note = john_client.get_note(request_note['note']['id'])
note = john_two_client.get_note(request_note['note']['id'])
assert note.content['status']['value'] == 'Accepted'

#Try to automatically automatically remove a reversed name
profile = john_client.get_profile('~John_Last1')
profile = john_two_client.get_profile('~John_Last2')
profile.content['names'].append(
{'first':'Last',
'last': 'John'}
)
john_client.post_profile(profile)
profile=john_client.get_profile('~John_Last1')
john_two_client.post_profile(profile)

request_note = john_client.post_note_edit(
## add a publication
john_two_client.post_note_edit(
invitation='openreview.net/Archive/-/Direct_Upload',
signatures=['~Last_John1'],
note = openreview.api.Note(
pdate = openreview.tools.datetime_millis(datetime.datetime(2019, 4, 30)),
content = {
'title': { 'value': 'Paper title 2' },
'abstract': { 'value': 'Paper abstract 2' },
'authors': { 'value': ['Last John'] },
'authorids': { 'value': ['~Last_John1'] },
'venue': { 'value': 'Arxiv' }
},
license = 'CC BY-SA 4.0'
))
profile=john_two_client.get_profile('~John_Last2')

request_note = john_two_client.post_note_edit(
invitation='openreview.net/Support/-/Profile_Name_Removal',
signatures=['~John_Last1'],
signatures=['~John_Last2'],
note = openreview.api.Note(
content={
'name': { 'value': 'Last John' },
Expand All @@ -651,11 +707,10 @@ def test_remove_alternate_name(self, openreview_client, test_client, helpers):
)
helpers.await_queue_edit(openreview_client, edit_id=request_note['id'])

note = john_client.get_note(request_note['note']['id'])
note = john_two_client.get_note(request_note['note']['id'])
assert note.content['status']['value'] == 'Accepted'



def test_remove_name_and_rename_profile_id(self, client, openreview_client, helpers):

ana_client = helpers.create_user('[email protected]', 'Ana', 'Last', alternates=[], institution='google.com')
Expand Down Expand Up @@ -950,58 +1005,58 @@ def test_remove_name_from_merged_profile(self, openreview_client, helpers):
assert len(publications) == 1


ella_client_2 = helpers.create_user('[email protected]', 'Ella', 'Last', alternates=[], institution='deepmind.com')
ella_client_2 = helpers.create_user('[email protected]', 'Ela', 'Last', alternates=[], institution='deepmind.com')

profile = ella_client_2.get_profile()
assert '~Ella_Last2' == profile.id
assert '~Ela_Last1' == profile.id

assert openreview_client.get_group('~Ella_Last2').members == ['[email protected]']
assert openreview_client.get_group('[email protected]').members == ['~Ella_Last2']
assert openreview_client.get_group('~Ela_Last1').members == ['[email protected]']
assert openreview_client.get_group('[email protected]').members == ['~Ela_Last1']

ella_client_2.post_note_edit(
invitation='openreview.net/Archive/-/Direct_Upload',
signatures=['~Ella_Last2'],
signatures=['~Ela_Last1'],
note = openreview.api.Note(
pdate = openreview.tools.datetime_millis(datetime.datetime(2019, 4, 30)),
content = {
'title': { 'value': 'Paper title 2' },
'abstract': { 'value': 'Paper abstract 2' },
'authors': { 'value': ['Ella Last', 'Test Client'] },
'authorids': { 'value': ['~Ella_Last2', '[email protected]'] },
'authors': { 'value': ['Ela Last', 'Test Client'] },
'authorids': { 'value': ['~Ela_Last1', '[email protected]'] },
'venue': { 'value': 'Arxiv' }
},
license = 'CC BY-SA 4.0'
))

publications = openreview_client.get_notes(content={ 'authorids': '~Ella_Last2'})
publications = openreview_client.get_notes(content={ 'authorids': '~Ela_Last1'})
assert len(publications) == 1


openreview_client.merge_profiles('~Ella_Last1', '~Ella_Last2')
openreview_client.merge_profiles('~Ella_Last1', '~Ela_Last1')
profile = ella_client.get_profile()
assert len(profile.content['names']) == 3
profile.content['names'][0]['username'] == '~Ella_Last1'
profile.content['names'][0]['preferred'] == True
profile.content['names'][1]['username'] == '~Ella_Alternate_Last1'
profile.content['names'][2]['username'] == '~Ella_Last2'
profile.content['names'][2]['username'] == '~Ela_Last1'

found_profiles = openreview_client.search_profiles(fullname='Ella Last', use_ES=True)
assert len(found_profiles) == 1
assert len(found_profiles[0].content['names']) == 3

assert openreview_client.get_group('~Ella_Last1').members == ['[email protected]', '[email protected]']
assert openreview_client.get_group('~Ella_Last2').members == ['[email protected]']
assert openreview_client.get_group('~Ela_Last1').members == ['[email protected]']
assert openreview_client.get_group('[email protected]').members == ['~Ella_Last1', '~Ella_Alternate_Last1']
assert openreview_client.get_group('[email protected]').members == ['~Ella_Last2', '~Ella_Last1']
assert openreview_client.get_group('[email protected]').members == ['~Ela_Last1', '~Ella_Last1']
assert openreview_client.get_group('~Ella_Alternate_Last1').members == ['[email protected]']

request_note = ella_client.post_note_edit(
invitation='openreview.net/Support/-/Profile_Name_Removal',
signatures=['~Ella_Last1'],
note = openreview.api.Note(
content={
'name': { 'value': 'Ella Last' },
'usernames': { 'value': ['~Ella_Last2'] },
'name': { 'value': 'Ela Last' },
'usernames': { 'value': ['~Ela_Last1'] },
'comment': { 'value': 'typo in my name' }
}
)
Expand All @@ -1013,7 +1068,7 @@ def test_remove_name_from_merged_profile(self, openreview_client, helpers):
assert len(messages) == 1
assert messages[0]['content']['text'] == '''Hi Ella Last,
We have received your request to remove the name "Ella Last" from your profile: https://openreview.net/profile?id=~Ella_Last1.
We have received your request to remove the name "Ela Last" from your profile: https://openreview.net/profile?id=~Ella_Last1.
We will evaluate your request and you will receive another email with the request status.
Expand Down Expand Up @@ -1057,8 +1112,8 @@ def test_remove_name_from_merged_profile(self, openreview_client, helpers):
assert len(found_profiles) == 1
assert len(found_profiles[0].content['names']) == 2

with pytest.raises(openreview.OpenReviewException, match=r'Group Not Found: ~Ella_Last2'):
openreview_client.get_group('~Ella_Last2')
with pytest.raises(openreview.OpenReviewException, match=r'Group Not Found: ~Ela_Last1'):
openreview_client.get_group('~Ela_Last1')

assert openreview_client.get_group('~Ella_Last1').members == ['[email protected]', '[email protected]']
assert openreview_client.get_group('[email protected]').members == ['~Ella_Last1', '~Ella_Alternate_Last1']
Expand All @@ -1069,7 +1124,7 @@ def test_remove_name_from_merged_profile(self, openreview_client, helpers):
assert len(messages) == 1
assert messages[0]['content']['text'] == '''Hi Ella Last,
We have received your request to remove the name "Ella Last" from your profile: https://openreview.net/profile?id=~Ella_Last1.
We have received your request to remove the name "Ela Last" from your profile: https://openreview.net/profile?id=~Ella_Last1.
The name has been removed from your profile. Please check that the information listed in your profile is correct.
Expand Down

0 comments on commit 2b03531

Please sign in to comment.