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

fix(Import): expect selectionOptions as array from json #1383

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Sep 25, 2024

currently an import fails silently (no web UI feedback) with Argument #15 ($selectionOptions) must be of type ?string, array given in the logs. This fixes handlig of the selectionOptions.

@blizzz blizzz added the 3. to review Waiting for reviews label Sep 25, 2024
@blizzz blizzz added the bug Something isn't working label Sep 25, 2024
@blizzz
Copy link
Member Author

blizzz commented Sep 25, 2024

We should check with the usergroup column types, too.

@@ -23,7 +23,7 @@ public function __construct(
private ?int $numberDecimals = null,
private ?string $numberPrefix = null,
private ?string $numberSuffix = null,
private ?string $selectionOptions = null,
private ?array $selectionOptions = null,
Copy link
Contributor

@enjeck enjeck Sep 27, 2024

Choose a reason for hiding this comment

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

Hmm, are we sure about this? I believe selectionOptions is a comma-delimited string. Maybe it's only shown as a string in the frontend, idk.

Copy link
Member Author

Choose a reason for hiding this comment

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

from my debugging yes, and the change worked locally.

@enjeck enjeck linked an issue Sep 28, 2024 that may be closed by this pull request
@luka-nextcloud
Copy link
Contributor

@blizzz My suggestion for fixing this issue is updating this line https://github.com/nextcloud/tables/blob/main/lib/Controller/ApiTablesController.php#L160 to
selectionOptions: $column['selectionOptions'] == [] ? '' : json_encode($column['selectionOptions']),
instead of current change.

Otherwise, we have to update the data type in both ColumnController and frontend
https://github.com/nextcloud/tables/blob/main/lib/Controller/ColumnController.php#L97
https://github.com/nextcloud/tables/blob/main/lib/Controller/ColumnController.php#L198
https://github.com/nextcloud/tables/blob/main/src/modules/modals/CreateColumn.vue#L328
There are many frontend codes that should be updated for this approach.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz force-pushed the fix/noid/import-selectionoptions branch from 7e80e04 to c288a3e Compare October 14, 2024 19:14
@blizzz
Copy link
Member Author

blizzz commented Oct 14, 2024

Changed to Luka's suggestion

@blizzz
Copy link
Member Author

blizzz commented Oct 14, 2024

/backport to stable0.8

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label Oct 14, 2024
@blizzz
Copy link
Member Author

blizzz commented Oct 17, 2024

@luka-nextcloud mind reviewing this one? 🙈

@blizzz blizzz merged commit 65fa94d into main Oct 17, 2024
61 checks passed
@blizzz blizzz deleted the fix/noid/import-selectionoptions branch October 17, 2024 13:50
@backportbot backportbot bot removed the backport-request Pending backport by the backport-bot label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't import back scheme with selection column
3 participants