Skip to content
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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

breezewish
Copy link
Member

@breezewish breezewish commented Aug 5, 2017

#230

  • Add option to change "default source code publicity" in user preference (@breeswish)
  • Allow overridding the publicity for specific submission when submitting [deprecated: user can change publicity after submission] (@breeswish)
  • Allow changing publicity for specific submission in submission_detail (@breeswish)
  • Check publicity according to new rules (@twd2)

Domain overriding publicity setting is intended to be out of this PR's scope.

@twd2 twd2 requested a review from iceboy233 August 12, 2017 18:45
@twd2 twd2 changed the title WIP: Support setting record publicity Support setting record publicity Aug 12, 2017
@twd2
Copy link
Member

twd2 commented Aug 12, 2017

Discuss: is EDIT_RECORD_VISIBILITY(_SELF) a perm or a priv?

@breezewish
Copy link
Member Author

I think "publicity" is more accurate than "visibility".

@breezewish
Copy link
Member Author

breezewish commented Aug 13, 2017

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.

@twd2
Copy link
Member

twd2 commented Aug 14, 2017

According to our design before, records are global. ^_^ T_T

@iceb0y

@twd2
Copy link
Member

twd2 commented Aug 15, 2017

@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
Copy link
Member

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 :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int52???

Copy link
Member Author

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},
Copy link
Member

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

@iceboy233
Copy link
Member

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.

@breezewish
Copy link
Member Author

breezewish commented Aug 23, 2017

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants