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

New Governance Process SlackAdvancedVote to the Slack Plugin #225

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

Conversation

leijie-wang
Copy link

The implementation is similar to SlackEmojiVote.

This new governance process accepts a list of candidates and options and then initiates a vote in Slack where users can select an option for each candidate. The outcome of the vote will be stored as a dict {user: {candidate1: option1, ...}, }

To allow the new governance to receive webhook events, in the handlers.py of Slack, we also check whether the actions of the payload are isVOTE_ACTION_ID or start with ADVANCED_VOTE_ACTION_ID), and pass events to the corresponding governance process.

Copy link
Collaborator

@amyxzhang amyxzhang left a comment

Choose a reason for hiding this comment

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

Overall looks good!

@@ -141,7 +141,7 @@ def quality_is_greater(a, b):
return order.index(a) > order.index(b)


class Plugin(models.Model):
class Plugin(models.Model ):
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

@@ -191,10 +191,9 @@ def start_process(self, process_name, callback_url=None, **kwargs):
"""Start a new GovernanceProcess"""
# Find the proxy class for the specified GovernanceProcess
cls = self.__get_process_cls(process_name)

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this?

ADVANCED_VOTE_ACTION_ID = "advanced_vote"

@Registry.governance_process
class SlackAdvancedVote(GovernanceProcess):
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of calling this advanced vote, since this is somewhat vague, what about calling it SlackMultichoiceVote or SlackElectionVote?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this class meant to represent different kinds of more advanced votes?

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.

2 participants