add rank voting feature merge chosenrank with 8.1.4 refactorings using variant 'generic' #4199
Conversation
reworked from git@github.com:vinimoz/polls.git rank-vote-feature-vue3 TRAPPE Vincent <trappe.vincent@laposte.Net> voting variant generic a configuration list of answers for a vote. Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
|
I surfed through the changes and made some comments. I did not do a complete review. But generally: The handling of the votes confuse me. In my opinion, the votes should be stored as votes and get the ranks as an answer; instead of yes|no|maybe. This way we can reuse a lot of code already there or extend further by adding other voting variants. |
| import CloseIcon from 'vue-material-design-icons/Close.vue' | ||
| import { showError } from '@nextcloud/dialogs' | ||
| const store = usePollStore() |
There was a problem hiding this comment.
Don't use store as a store name. Use the store name instead:
pollStore = usePollStore()
| protected string $access = ''; | ||
| protected int $anonymous = 0; | ||
| protected int $allowMaybe = 0; | ||
| protected string $chosenRank = ''; |
There was a problem hiding this comment.
I still do not get the purpose of chosenRank.
| * @psalm-return array{0: string, 1: string} | ||
| */ | ||
| private function getValidVotingVariant(): array { | ||
| return [Poll::VARIANT_SIMPLE, Poll::VARIANT_GENERIC]; |
There was a problem hiding this comment.
- The 'generic variant' should be an 'ordered variant' or VARIANT_ORDERED in this case
| active?: boolean | ||
| } | ||
| const { answer, active = false } = defineProps<Props>() |
There was a problem hiding this comment.
Nope. Props get destructed! keep the existing code.
| export type VoteVariant = 'simple' | ||
| export const votingVariants: Record<VotingVariant, VotingVariantsType> = { | ||
| simple: { | ||
| name: t('polls', 'Simple variant'), |
There was a problem hiding this comment.
The "simple variant" could be named as "opted variant" or simple "yes|no|maybe"
| </div> | ||
| </template> | ||
|
|
||
| <script setup> |
There was a problem hiding this comment.
You introduced ConfigRankOptions.vue. What is the difference from the current options configuration?
|
And you should make a rebase from the master branch. |
address remarks from nextcloud#4199 Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
address remarks from nextcloud#4199 Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
address remarks from nextcloud#4199 Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Can this be closed? |
yes. |
address remarks from nextcloud#4199 Signed-off-by: philippe lhardy <philippe.lhardy@astrolabe.coop>
2 participants
Add a new 'generic' voting variant to replace generic poll type based on @vinimoz rank-vote-feature-vue3 branch.
Variant is another choice than Poll type :
Works with both Date and Text.
There is no result displayed, cumul of votes is not (yet) implemented for generic variant.
This PR is intended to be merged into at v8.1.4 tag, but can't find related branch in PR request GUI,
I am still working on a merge in main too.