-
Notifications
You must be signed in to change notification settings - Fork 83
Votes rank feature enabled #3930
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
Conversation
dartcafe
left a comment
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.
First quick look.
lib/Db/Poll.php
Outdated
| public const TYPE_TEXT = 'textPoll'; | ||
| public const TYPE_IND_TEXT = 'textIndPoll'; | ||
| public const TYPE_RANK_TEXT = 'textRankPoll'; |
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.
I would prefer leaving the constants unchanged. Don't change the constants. Just add the new one (TYPE_RANKED_TEXT).
Or better TYPE_TEXT_RANKED
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.
if it doesnt mind u i've keep TYPE_RANKED_POOL like my field is textRankPool and as i'm lazy
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.
It is because of sorting and hierarchical context. At first it is a text poll and second there is a voting variation.
That is not important, but changing the existing types is no good idea. Already existing polls are marked as textPoll inside the database.
Leaving the existent constants name results in less code changes to review.
lib/Db/Poll.php
Outdated
| protected string $access = ''; | ||
| protected int $anonymous = 0; | ||
| protected int $allowMaybe = 0; | ||
| protected $choosenRank = []; |
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.
Avoid array as database type. I am not sure, if it work with all db engines properly.
Prefer a string and add getters which decode and encode the string into an array.
BTW: typo choosen => chosen
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.
ya right. I've change it to string i will test the modification.
| @@ -0,0 +1,171 @@ | |||
| <template> | |||
| <div class="option-container"> | |||
| <!-- Menu déroulant pour sélectionner une option --> | |||
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.
Please. Comments always in English language!
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.
yep
|
Sorry, but i'm a newbie, in github, vue, etc 😄 . I've proceed with modifications and fix. I've try several time to put in the branch master-7, Rank votes features, without luck, i always get access denied. I will put the update on my fork |
|
After several tries, i manage to do a commit with the changes. I can see my commit. On the commit. I was trying to change the mode draft but i think i cannot do that. |
Don't care. That's the way I went through, when I started here. |
You can't change it, because you are not a member of the nextcloud organisation (organisation here on Github). All changes are done inside your branch and end here in this PR as soon as you commit them to your branch (in your repo) |
|
@vinimoz To make the changes better readable please run You could also run eslint ( |
|
Yesterday, i commit and other on. eslint doenst give me any more error. Regarding phpcs and php-cs for lib directory |
d2b16e6 to
a918de1
Compare
Signed-off-by: Hamza Mahjoubi <hamzamahjoubi221@gmail.com>
This reverts commit 46aa777. Signed-off-by: dartcafe <github@dartcafe.de>
This reverts commit c5a55ab. Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: vinimoz <trappe.vincent@laposte.net>
- Remove modification of textPoll to textIndPoll - Change of array to string - Updates translation - Some fix Signed-off-by: vinimoz <trappe.vincent@laposte.net>
Signed-off-by: vinimoz <trappe.vincent@laposte.net>
…nt fix Signed-off-by: vinimoz <trappe.vincent@laposte.net>
a918de1 to
007a63e
Compare
|
something is not clean here. You commited reverted commits from the master-7 branch (affecting the translation files). How did you create your branch? |
|
Oups sorry, maybe it's my fault it coz someone else was working on my
branch of the fork, Rank_votes_feature ?.
Yesterday, i try to commit on the pr3930, as i was seeing my commit
there. After realize i haven't the right, after go back on the correct
place, i do a mistake a create a branch 3930_commit on my main branch by
error. So i after i merge it with Rank_vote s_feature and delete it the
3930_commit. After i've tried to fight to fix all DCO.
To create my branch originally after the fork i do, after i remember, i
clone it, fetch the upstream, i do gitcheckout -b Rank_votes_feature.
Its what i remember.
…On 27/03/2025 09:09, René Gieling wrote:
something is not clean here.
You commited reverted commits from the master-7 branch (affecting the
translation files). How did you create your branch?
—
Reply to this email directly, view it on GitHub
<#3930 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW7C5TTF4CFZBE25ZDVVVV32WQ5HBAVCNFSM6AAAAABZVOBHUSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJZGE2DGMBZGA>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
dartcafe*dartcafe* left a comment (nextcloud/polls#3930)
<#3930 (comment)>
something is not clean here.
You commited reverted commits from the master-7 branch (affecting the
translation files). How did you create your branch?
—
Reply to this email directly, view it on GitHub
<#3930 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW7C5TTF4CFZBE25ZDVVVV32WQ5HBAVCNFSM6AAAAABZVOBHUSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONJZGE2DGMBZGA>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@vinimoz Are you still working on this? I would prefer to close this PR out of the following reasons:
|
|
I’ve stopped working on it since, u told me something wrong happens,
I had modified some files that ideally shouldn’t have been changed. That
was my understanding up to that point.
That said, throughout the development, I made an effort to keep changes
to a minimum and maintain the flexibility you had implemented for the
other voting types. With the current setup, we can proceed with votes,
smiley like, score voting, letter grades, majority judgment, Borda,
Nauru, and Condorcet. I do agree that a drag-and-drop feature would
improve usability, especially for Condorcet, and could definitely be
refined.
Regarding the version, I was aware of the situation. That’s one of the
reasons I aimed to deliver a usable version as quickly as possible to
avoid blocking progress. Migrating to Vue 3 shouldn't be a major issue.
Also, I needed those improvements fairly quickly for our political group.
I understand your point of view. What I don’t quite understand is why I
was asked to fix or change things if, in the end, it wasn’t what you
were looking for. It would’ve simply been more transparent and
respectful to clarify that from the beginning.
Le 21/04/2025 à 06:01, René Gieling a écrit :
…
@vinimoz <https://github.com/vinimoz> Are you still working on this?
I would prefer to close this PR out of the following reasons:
* This PR is based on v7 and v7 will slowly become legacy, since the
v8 is nearly about to leave beta state
* v8's code base is not compatible to v7's, so any changes will have
a short life
* I already prepared voting variants here
<#3973>
A ranked voting is rather a voting variant but a poll type.
* We should think the implementation first from the UI.
I prefer another UI for ranked votes (something like moving
options by drag and drop into an order instead of entering numbers)
—
Reply to this email directly, view it on GitHub
<#3930 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW7C5TXQGKZKG53OZ7JBIVT22UI5ZAVCNFSM6AAAAABZVOBHUSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMJYHA4TGMZQGY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
*dartcafe* left a comment (nextcloud/polls#3930)
<#3930 (comment)>
@vinimoz <https://github.com/vinimoz> Are you still working on this?
I would prefer to close this PR out of the following reasons:
* This PR is based on v7 and v7 will slowly become legacy, since the
v8 is nearly about to leave beta state
* v8's code base is not compatible to v7's, so any changes will have
a short life
* I already prepared voting variants here
<#3973>
A ranked voting is rather a voting variant but a poll type.
* We should think the implementation first from the UI.
I prefer another UI for ranked votes (something like moving
options by drag and drop into an order instead of entering numbers)
—
Reply to this email directly, view it on GitHub
<#3930 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW7C5TXQGKZKG53OZ7JBIVT22UI5ZAVCNFSM6AAAAABZVOBHUSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMJYHA4TGMZQGY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
No description provided.