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

Handle non-public contribution types #3138

Merged
merged 1 commit into from Nov 21, 2017

Conversation

mic4ael
Copy link
Member

@mic4ael mic4ael commented Nov 1, 2017

No description provided.

@mic4ael mic4ael changed the title Add non-public contribution types Handle non-public contribution types Nov 1, 2017
Copy link
Member

@ThiefMaster ThiefMaster left a comment

Choose a reason for hiding this comment

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

Please also add a changelog entry (I think it fits best under "Improvements", we can always move it to "Major Features" later if needed):

- Allow marking contribution types as private so they cannot be selected
  by users submitting an abstract (:issue:`3138`)

def upgrade():
op.add_column(
'contribution_types',
sa.Column('is_private', sa.Boolean(), nullable=False, server_default=sa.true()),
Copy link
Member

Choose a reason for hiding this comment

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

server_default='true' would be enough, but nothing wrong with letting SA generate that string for you

@@ -474,7 +474,10 @@ def __init__(self, *args, **kwargs):
if abstracts_settings.get(self.event, 'contrib_type_required'):
inject_validators(self, 'submitted_contrib_type', [DataRequired()])
super(AbstractForm, self).__init__(*args, **kwargs)
self.submitted_contrib_type.query = self.event.contribution_types
if self.event.can_manage(session.user):
Copy link
Member

Choose a reason for hiding this comment

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

I would use the management argument that's available in make_abstract_form for this (pass it to the form constructor). No need to check the event ACL again.

@@ -187,6 +188,9 @@ class ContributionTypeForm(IndicoForm):
"""Form to create or edit a ContributionType"""

name = StringField(_("Name"), [DataRequired()])
is_private = BooleanField(_("Private contribution type"), widget=SwitchWidget(), default=False,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "Private" or "Internal" is a better title? "Private contribution type" sounds a lot like something that'll be wrapped on two lines..

@@ -187,6 +188,9 @@ class ContributionTypeForm(IndicoForm):
"""Form to create or edit a ContributionType"""

name = StringField(_("Name"), [DataRequired()])
is_private = BooleanField(_("Private contribution type"), widget=SwitchWidget(), default=False,
description=_("If unchecked, the contribution type can be assigned by managers/judges "
Copy link
Member

Choose a reason for hiding this comment

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

"If selected, this contribution type cannot be selected by users submitting an abstract."?

Copy link
Member

Choose a reason for hiding this comment

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

"selected" twice sounds a bit weird. Maybe ".. cannot be chosen by users...?

@@ -2,6 +2,7 @@
<tr class="i-table">
<td class="i-table name-column">{{ contrib_type.name }}</td>
<td class="i-table">{{ contrib_type.description }}</td>
<td class="i-table">{{ 'Yes' if contrib_type.is_private else 'No' }}</td>
Copy link
Member

Choose a reason for hiding this comment

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

_('Yes') and _('No')

if self.event.can_manage(session.user):
self.submitted_contrib_type.query = self.event.contribution_types
else:
self.submitted_contrib_type.query = self.event.contribution_types.filter_by(is_private=False)
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 not covering that case where you are editing an existing abstract that has a private type. I think this might do the job:

criteria = [~ContributionType.is_private]
if self.abstract and self.abstract.submitted_contrib_type:
    criteria.append(ContributionType.id == self.abstract.submitted_contrib_type.id)
self.submitted_contrib_type.query = self.event.contribution_types.filter(db.or_(*criteria))

@mic4ael mic4ael force-pushed the non-public-contribution-types branch 2 times, most recently from cc46ae3 to 72533c7 Compare November 1, 2017 14:08
@mic4ael
Copy link
Member Author

mic4ael commented Nov 1, 2017

updated

@@ -187,6 +188,9 @@ class ContributionTypeForm(IndicoForm):
"""Form to create or edit a ContributionType"""

name = StringField(_("Name"), [DataRequired()])
is_private = BooleanField(_("Private"), widget=SwitchWidget(), default=False,
Copy link
Member

Choose a reason for hiding this comment

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

default=False isn't needed

@mic4ael mic4ael force-pushed the non-public-contribution-types branch 2 times, most recently from 317ec4e to 17072b9 Compare November 1, 2017 14:35
@mic4ael
Copy link
Member Author

mic4ael commented Nov 2, 2017

updated

@ThiefMaster ThiefMaster added this to the v2.1 milestone Nov 6, 2017
@mic4ael mic4ael dismissed ThiefMaster’s stale review November 9, 2017 12:38

Added changelog entry

@mic4ael mic4ael force-pushed the non-public-contribution-types branch from 17072b9 to 6dcfe47 Compare November 16, 2017 14:56
@mic4ael mic4ael force-pushed the non-public-contribution-types branch 2 times, most recently from 9d2eac5 to 6579b7d Compare November 17, 2017 14:09
@@ -474,7 +474,13 @@ def __init__(self, *args, **kwargs):
if abstracts_settings.get(self.event, 'contrib_type_required'):
inject_validators(self, 'submitted_contrib_type', [DataRequired()])
super(AbstractForm, self).__init__(*args, **kwargs)
self.submitted_contrib_type.query = self.event.contribution_types
if kwargs['management']:
Copy link
Member

Choose a reason for hiding this comment

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

management = kwargs.pop('management')

before calling super (i'd put it after setting self.abstract); then use management here. no need to pass along this kwarg to the superclass.

also, it sounds like something where a default value could make sense =>

management = kwargs.pop('management', False)

@mic4ael mic4ael force-pushed the non-public-contribution-types branch from 6579b7d to 796dc97 Compare November 21, 2017 13:53
@ThiefMaster ThiefMaster merged commit 796dc97 into indico:v2.1-dev Nov 21, 2017
@mic4ael mic4ael deleted the non-public-contribution-types branch November 21, 2017 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants