Variant generic unified main

[artlog]

Variant Generic adapted/merged to main Following #4199

Variant generic is another choice than Poll type where chosen_rank is a json array of possible configurable text answers from poll creator/owner.

creating

selecting possible answers at creation time in configuration tab :

then polls are selectable

Currently used to be exported and computed outside nextcloud poll with csv, results are not visible nor computed in poll.

VoteVariant type in front was renamed VotingVariant to match php code naming. Choice to use VoteVariant into php could have been done too, prefering to keep api side naming. Unification seems needed for Api poll calls.

Hoping this is clearer now.

[dartcafe]

VoteVariant type in front was renamed VotingVariant to match php code naming. Choice to use VoteVariant into php could have been done too, prefering to keep api side naming. Unification seems needed for Api poll calls.

Yes. Should be consistent. Changing the backend naming would affect the database, too. So this decision was the better one.

Hoping this is clearer now.

Yep! :+1:

[dartcafe]

@artlog. I see you approach the final solution in steps. I like that.

So when ever you think a package is ready, we can merge step by step and give the feature a toggle, so the functionality can be constantly tested without releasing unfinished features.

What do you think?

[artlog]

this is the idea. step by step...

[artlog]

To have a coherent functionality quickly i study this addition

Voting variant generic is disabled by default and should be activated globally by administrator. So there is no visible change at upgrade without an explicit action to enable variant.

[dartcafe]

@artlog The merge conflict can be solved easily: Just accept both changes.

Not that we are waiting for each other: If you want to merge, request a review by me as a sign, that you are finished with this PR from your perspective.

[dartcafe]

CI looks good so far

Some to dos:

• Add the new property to the test factory (tests/Unit/Factories/PollFactory.php)
• run composer run cs:fix
• Do a rebase and resolve the merge conflict (accept both changes)

[artlog]

i just aligned existing code with current main and test request, no new change.

Current code add variant generic for which usage is for collecting votes only. Results should be exported and computed outside nextcloud poll with csv, results are not visible nor computed in poll.

Majority judgement new method won't be done within this merge, but this will be required to provide it,

This does not include creation of new variant disabled by default from my prior comment, i will notify when done.

[dartcafe]

@artlog If you make a rebase: TableSchema has moved to a version sub dir. I fear you have to solve this conflict manually. As far as I can see, the only change is the addition of your new field to polls.

[artlog]

@dartcafe this is working on my system, it look like TableSchema change was already merged, did it ?

I had to reset my environment since i got a Not null violation on group_id oc_polls_share , this is working a clean env on 31.0.6 nextcloud version.

• rebased on main
• added variants selection disabled by default require to be enable in user settings of poll.

This was my final change for this, hoping you get this validated soon ;-)

[dartcafe]

Some things left:

• sign your commits (DCO fails, click on the entry and see how to do it)
• format/prettier fix (Lint prettier fails)
• ~~add the new polls property to the test factory~~ (PHPUnit fails) Nonsense: It is set. Maybe it is the default = 1 in the TableSchema for the text type, what is confusing the test
• Add missing copyright info (REUSE Check fails)
• request a review from me, when finished

[dartcafe]

Not null violation on group_id oc_polls_share

This was probably #4213

[artlog]

@dartcafe hopefully fixed comments requests

[dartcafe]

Let's see, what CI says. :wink:

[artlog]

it is a little disapointing... what that's default problem. looks like all other Types:TEXT are nullable... ( 'notnull' => false, 'default' => null ) while this one is not. will see tomorrow.

[dartcafe]

it is a little disapointing... what that's default problem.

I am wondering, too. I'll have a look at it. I don't know, what I don't see.

[dartcafe]

looks like all other Types:TEXT are nullable... ( 'notnull' => false, 'default' => null ) while this one is not. will see tomorrow.

Umpf. :see_no_evil: Yes, I think this is the problem, because a text field with default '' is treated as null. If I remember right, it is because of Oracle DB, which has this limitation.

This means chosen_rank has to be nullable.

Quick search: https://www.sqlservercentral.com/forums/topic/oracle-empty-string-is-the-same-as-null But a lot of other articles can be found

Usually it should be reported by the migrationManager https://github.com/nextcloud/server/blob/23573c4947a2d9a5db9907a76eb4151985ee6542/lib/private/DB/MigrationService.php#L571-L575

Same problem applies to boolean fields. Obviously Oracle says null = false.

[github-actions[bot]]

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

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

[artlog]

i kept '' default value but set it as nullable, hoping this is the good choice.

[dartcafe]

Code-wise it looks good to me so far. :tada: I will test the changes this evening or the next days.

[dartcafe]

Just checked out this PR. ATM all is broken, even existing polls.

I get in the console

Uncaught (in promise) SyntaxError: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data
    setup VoteButton.vue:67
    callWithErrorHandling runtime-core.esm-bundler.js:210

And following these when trying to navigate (which I assume broke the app and is just a following error):

Uncaught (in promise) TypeError: can't access property "nextSibling", node is null
    nextSibling runtime-dom.esm-bundler.js:64

access property "subTree", vnode.component is null
    getNextHostNode runtime-core.esm-bundler.js:6036

and more...

[artlog]

This was working on postgres and sqlite with non nullable, adding nullable to get it for mysql broke it on postgres. i don't have a mysql env and this is a pain if i should. Will see later...

[dartcafe]

This was working on postgres and sqlite with non nullable, adding nullable to get it for mysql broke it on postgres. i don't have a mysql env and this is a pain if i should. Will see later...

And I mainly use MySql. I know the pain. The db engines have different surprises. I can send you a picture of by teeth prints on my table. :rofl:

[dartcafe]

I patched the poll and added some generic options. This eliminates the error for the poll with generic options, but it stays for existing polls with the usual answers.

[artlog]

I patched the poll and added some generic options. This eliminates the error for the poll with generic options, but it stays for existing polls with the usual answers.

how do we proceed ? Can you provide your changes to start from that ? i will install a mariadDB, hoping it really acts the same as a MySql.

[dartcafe]

I patched the poll and added some generic options. This eliminates the error for the poll with generic options, but it stays for existing polls with the usual answers.

I was referring to the runtime error, when chosenRank was an empty string. I just added some voting options to the field in the db.

The CI error is probably caused here

[artlog]

I patched the poll and added some generic options. This eliminates the error for the poll with generic options, but it stays for existing polls with the usual answers.

I was referring to the runtime error, when chosenRank was an empty string. I just added some voting options to the field in the db.

The CI error is probably caused here

i will try in following days to support correctly nullable since it is actualy the case, chosenRank in standard polls is just empty anyway. and see for array, since it is really an array when used.

[artlog]

@dartcafe let's give a try like it is now ?

[artlog]

@dartcafe : after refereshing my environment with last merge and running occ upgrade i get this error :

Error: Typed property OCA\Polls\Db\V2\DbManager::$schema must not be accessed before initialization in /var/www/html/custom_apps/polls/lib/Db/V2/TableManager.php:466
Stack trace:
#0 /var/www/html/custom_apps/polls/lib/Migration/Version080301Date20250822153002.php(84): OCA\Polls\Db\V2\TableManager->fixNullishShares()
#1 /var/www/html/lib/private/DB/MigrationService.php(499): OCA\Polls\Migration\Version080301Date20250822153002->preSchemaChange(Object(OC\Migration\SimpleOutput), Object(Closure), Array)
#2 /var/www/html/lib/private/DB/MigrationService.php(395): OC\DB\MigrationService->executeStep('080301Date20250...', false)
#3 /var/www/html/lib/private/legacy/OC_App.php(694): OC\DB\MigrationService->migrate()
#4 /var/www/html/lib/private/Updater.php(325): OC_App::updateApp('polls')
#5 /var/www/html/lib/private/Updater.php(236): OC\Updater->doAppUpgrade()
#6 /var/www/html/lib/private/Updater.php(100): OC\Updater->doUpgrade('31.0.6.2', '31.0.6.2')
#7 /var/www/html/core/Command/Upgrade.php(192): OC\Updater->upgrade()
#8 /var/www/html/3rdparty/symfony/console/Command/Command.php(326): OC\Core\Command\Upgrade->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/3rdparty/symfony/console/Application.php(1078): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /var/www/html/3rdparty/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand(Object(OC\Core\Command\Upgrade), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/3rdparty/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/lib/private/Console/Application.php(187): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/console.php(87): OC\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput))
#14 /var/www/html/occ(33): require_once('/var/www/html/c...')

this is related to #4225

[dartcafe]

Yep. I accessed thew table schema to check for the existence of a column, but forgot to initialize it.

[artlog]

@dartcafe what is status of this, will it be integrated or what is expected from me ? BTW i am at berlin community nextcloud conference until 2th october 2025, if you are around.. i will get some community help here too.