-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test using different characters in role names #2367
base: master
Are you sure you want to change the base?
Conversation
Good idea! |
edge browser is failing because of url encoding |
please try openreview/openreview-web#2115 |
@@ -1723,7 +1723,7 @@ def pretty_id(group_id): | |||
transformed_token=re.sub('\..+', '', token).replace('-', '').replace('_', ' ') | |||
letters_only=re.sub('\d|\W', '', transformed_token) | |||
|
|||
if letters_only != transformed_token.lower(): | |||
if not (letters_only == letters_only.lower() and letters_only != letters_only.upper()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar change as in openreview/openreview-web#2115
@@ -994,6 +994,7 @@ def set_meta_review_invitation(self): | |||
if self.venue.use_senior_area_chairs: | |||
# Build SAC acronym | |||
sac_acronym = ''.join([s[0].upper() for s in self.venue.senior_area_chairs_name.split('_')]) | |||
sac_acronym = sac_acronym if sac_acronym.isascii() else 'SAC' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think sometimes acronym may not be meaningful
@@ -1089,7 +1089,8 @@ def setup_invite_assignment(self, hash_seed, assignment_title=None, due_date=Non | |||
'hash_seed': { 'value': hash_seed, 'readers': [ venue.venue_id ]}, | |||
'email_template': { 'value': email_template if email_template else ''}, | |||
'is_reviewer': { 'value': True if (self.match_group.id.split('/')[-1] in venue.reviewer_roles) else False }, | |||
'is_ethics_reviewer': { 'value': True if self.match_group.id == venue.get_ethics_reviewers_id() else False } | |||
'is_ethics_reviewer': { 'value': True if self.match_group.id == venue.get_ethics_reviewers_id() else False }, | |||
'reviewer_name': { 'value': venue.get_reviewers_name(pretty=False) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
save in content so venue.py can use it in ln1423
icml test should pass with this pr and openreview/openreview-web#2117 |
This is only for testing, not meant to be merged.
Changes the role names to use Japanese characters. Currently breaks around Line 1676 when asserting the edge browser url.
Update:
Commented out edge browser url assertions because the url encoding is different for these characters.
Now the tests break at Line 1681. Not sure why yet, I could have missed a name replacement or there could be another issue. I think it should be tested further.