Skip to content

Enhancement: Update rows via import#1824

Merged
enjeck merged 1 commit into
mainfrom
feature/allow-update-rows-via-import
Sep 24, 2025
Merged

Enhancement: Update rows via import#1824
enjeck merged 1 commit into
mainfrom
feature/allow-update-rows-via-import

Conversation

@Koc

@Koc Koc commented May 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR adds possibility to update rows via import. Closes #1403

🗒️ TODO

  • Automatically select "ID (Meta)" column
  • Add new tests

🎥 Preview

nextcloud-tables-2025-05-28_00.13.09.mp4

@Koc Koc requested review from blizzz and enjeck as code owners May 27, 2025 14:49
@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 3 times, most recently from 7d9820e to 35c3a82 Compare May 27, 2025 22:11
@blizzz blizzz added enhancement New feature or request 2. developing Work in progress labels Jun 11, 2025
AIlkiv
AIlkiv requested changes Jun 25, 2025
View reviewed changes

@AIlkiv AIlkiv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for your work. I have a few comments

Comment thread lib/Service/ImportService.php Outdated
Comment thread lib/Service/ImportService.php Outdated
@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 14 times, most recently from 82651fc to 9ebf499 Compare July 8, 2025 23:59
@Koc

Koc commented Jul 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Any ideas why new scenario not pass for NC 29?

    Given file "update-rows.csv" exists for user "participant1" with the following data    # FeatureContext::createCsvFile()
      | ID      | one         | two |
      | {rowId} | AHA updated | 99  |
      |         | new row     | 100 |
      Failed asserting that 401 matches expected 201.

at the same time it works fine for all other versions. I've inspired from already existent Given user :user uploads file :file which works on NC29 as well

@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 3 times, most recently from 1a7dd57 to 47c840c Compare July 18, 2025 21:21
@enjeck enjeck mentioned this pull request Jul 23, 2025
Open
enjeck
enjeck reviewed Jul 23, 2025
View reviewed changes

@enjeck enjeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be great to have Cypress tests as well, considering there are so many changes

@AIlkiv AIlkiv self-requested a review July 23, 2025 07:53
@Koc Koc mentioned this pull request Jul 27, 2025
Merged
4 tasks
@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 6 times, most recently from 20c539d to 7774cc2 Compare August 3, 2025 16:25
@Koc

Koc commented Aug 3, 2025

Copy link
Copy Markdown
Contributor Author

@enjeck I've added Cypress test for this new functionality

Component tests failure is not related to my changes. It fails everywhere.

@enjeck enjeck force-pushed the feature/allow-update-rows-via-import branch from 7774cc2 to ab21bac Compare August 12, 2025 03:03
enjeck
enjeck requested changes Aug 12, 2025
View reviewed changes

@enjeck enjeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was just testing and most of it works. Except when after I import and preview, I choose to "Create new column" instead of "Import to existing column". When I do that I click "Import", it doesn't work. I'm stuck in a loop in same Preview modal.
Also, the edit pencil icon against the "ID" column does not work when in new mode like other column types 🤔

@enjeck

enjeck commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

When I do that I click "Import", it doesn't work

I think the import does create the new column, but the modal doesn't disappear like it should

@enjeck

enjeck commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

Plus, after using this ID as a new column, the original table no longer loads. Here's what apps/tables/api/1/tables/1/columns returns:

[
    {
        "id": 1,
        "tableId": 1,
        "title": "What",
        "createdBy": "admin",
        "createdByDisplayName": "admin",
        "createdAt": "2025-08-12 01:59:41",
        "lastEditBy": "admin",
        "lastEditByDisplayName": "admin",
        "lastEditAt": "2025-08-12 01:59:41",
        "type": "text",
        "subtype": "line",
        "mandatory": false,
        "description": "",
        "numberDefault": null,
        "numberMin": null,
        "numberMax": null,
        "numberDecimals": 0,
        "numberPrefix": "",
        "numberSuffix": "",
        "textDefault": null,
        "textAllowedPattern": null,
        "textMaxLength": null,
        "selectionOptions": [],
        "selectionDefault": null,
        "datetimeDefault": null,
        "usergroupDefault": [],
        "usergroupMultipleItems": false,
        "usergroupSelectUsers": false,
        "usergroupSelectGroups": false,
        "usergroupSelectTeams": false,
        "showUserStatus": false
    },
...
    {
        "id": 14,
        "tableId": 1,
        "title": "ID (2)",
        "createdBy": "admin",
        "createdByDisplayName": "admin",
        "createdAt": "2025-08-12 03:24:34",
        "lastEditBy": "admin",
        "lastEditByDisplayName": "admin",
        "lastEditAt": "2025-08-12 03:24:34",
        "type": "-1",
        "subtype": "",
        "mandatory": false,
        "description": "This column was automatically created by the import service.",
        "numberDefault": null,
        "numberMin": null,
        "numberMax": null,
        "numberDecimals": 0,
        "numberPrefix": "",
        "numberSuffix": "",
        "textDefault": null,
        "textAllowedPattern": null,
        "textMaxLength": null,
        "selectionOptions": [],
        "selectionDefault": null,
        "datetimeDefault": null,
        "usergroupDefault": [],
        "usergroupMultipleItems": false,
        "usergroupSelectUsers": false,
        "usergroupSelectGroups": false,
        "usergroupSelectTeams": false,
        "showUserStatus": false
    }
]|

Notice that for the last newly-added column, the type is -1, and ourr frontend parsing code does not support this. This breaks the original Table and it's stuck loading. The console gives the error:

Uncaught (in promise) Error: -1 is not a valid column type!
    at parseCol (columnParser.js:38:17)
    at data.js:71:40
    at Array.map (<anonymous>)
    at Object.getColumnsFromBE (data.js:71:29)
    at async Object.loadColumnsFromBE (data.js:77:21)
    at async VueComponent.reload (main.js:22:2)

@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 2 times, most recently from 7b32b8a to 5ee324c Compare August 15, 2025 21:16
@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 2 times, most recently from 9fabafe to 2614fbe Compare September 13, 2025 22:13
@Koc

Koc commented Sep 13, 2025

Copy link
Copy Markdown
Contributor Author

@enjeck good catch!
I've fixed this issue. Now it works properly for both scenarios: create new column and insert rows duplicates; reuse id column and use id for update

@Koc Koc force-pushed the feature/allow-update-rows-via-import branch 3 times, most recently from 7d9cd09 to 623a4e8 Compare September 17, 2025 08:26
@Koc @enjeck
Enhancement: Update rows via import
72371de 
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@enjeck enjeck force-pushed the feature/allow-update-rows-via-import branch from 623a4e8 to 72371de Compare September 24, 2025 05:10
enjeck
enjeck approved these changes Sep 24, 2025
View reviewed changes

@enjeck enjeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks!

@enjeck enjeck merged commit 29f7942 into main Sep 24, 2025
60 of 64 checks passed
@enjeck enjeck deleted the feature/allow-update-rows-via-import branch September 24, 2025 07:59
@joshtrichards joshtrichards mentioned this pull request Feb 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AIlkiv AIlkiv AIlkiv approved these changes

@enjeck enjeck enjeck approved these changes

@blizzz blizzz Awaiting requested review from blizzz blizzz is a code owner

Assignees

No one assigned

Labels

2. developing Work in progress enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Export/import data duplicate row instead of updating it (even with "ID" column left untouched)

4 participants

@Koc @enjeck @AIlkiv @blizzz