-
Notifications
You must be signed in to change notification settings - Fork 108
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
Support setting record publicity #255
base: master
Are you sure you want to change the base?
Conversation
Discuss: is EDIT_RECORD_VISIBILITY(_SELF) a perm or a priv? |
I think "publicity" is more accurate than "visibility". |
EDIT_RECORD_VISIBILITY(_SELF) should be a permission since the domain owner may want to set some record's source code to public for others to refer to, for example, after a contest. inference: records should be assigned a domain? domain-scoped records have many advantages, for example, allowing domain owner to view all records. |
According to our design before, records are global. ^_^ T_T @iceb0y |
@iceb0y ptal |
@@ -71,6 +71,10 @@ | |||
PERM_EDIT_TRAINING = 1 << 48 | |||
PERM_EDIT_TRAINING_SELF = 1 << 49 | |||
|
|||
# Record. | |||
PERM_EDIT_RECORD_VISIBILITY = 1 << 50 | |||
PERM_EDIT_RECORD_VISIBILITY_SELF = 1 << 51 |
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.
This is near the limit of int52. Please be careful :)
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.
int52???
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.
@iceb0y Shall we store permission in binary format in MongoDB? We could serialize a long int to binary via something like xx.to_bytes(16, byteorder='little')
if visibility not in constant.setting.SUBMISSION_VISIBILITY_RANGE: | ||
raise error.ValidationError('visibility') | ||
coll = db.coll('record') | ||
doc = await coll.find_one_and_update(filter={'_id': rid}, |
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.
doc is not used, please either remove the variable definition, or return something
Looks like this change makes the visibility mutable (can be changed after a record is created) and doesn't provide an option to set this on submission time. I think we should first allow user to set this on submission time but disallow user to change this once record is created. |
@iceb0y I think providing an extra option for overriding visibility might be not helpful and is a bit confusion for most of the users. It is a special use case so that it is not provided in the submission page. However users are more likely to change the visibility after submission though, because of knowing whether the submission is Accepted or not. We could restrict the mutation for submissions older than a specific time (i.e. one month) to prevent history db records being modified. This restriction still meet demands for new submissions and it only affects users who wish to change the visibility for history submissions. |
#230
Domain overriding publicity setting is intended to be out of this PR's scope.