Skip to content

Enhancement: add a read-only option to view columns - #1958

Merged
enjeck merged 3 commits into
mainfrom
feature/readonly-columns-v2
Sep 7, 2025
Merged

Enhancement: add a read-only option to view columns#1958
enjeck merged 3 commits into
mainfrom
feature/readonly-columns-v2

Conversation

@Koc

@Koc Koc commented Jul 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #1154 and replaces #1916. This PR contains complete implementation for the both BE and FE sides.

🔍 Preview

nextcloud-readonly-tables-2025-07-31_00.39.34.mp4

🚧 Todo:

  • Tests

@Koc
Koc requested review from blizzz and enjeck as code owners July 27, 2025 16:28
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch 2 times, most recently from 00fec50 to 2696c27 Compare July 27, 2025 16:35
@Aveyron-RetD

Aveyron-RetD commented Jul 28, 2025

Copy link
Copy Markdown

1000 thanks

Would it not be possible that in edition mode those columns do appear but they are greyed out and cannot be entered in. By removing them from edition here you might not know what you are editing.

@Koc Koc mentioned this pull request Jul 28, 2025
Closed
2 tasks
@Koc
Koc requested a review from AIlkiv July 28, 2025 14:27
@blizzz
blizzz requested a review from marcoambrosini July 29, 2025 13:58
@blizzz blizzz added enhancement New feature or request 3. to review Waiting for reviews labels Jul 29, 2025
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch 3 times, most recently from 433c9d5 to e035173 Compare July 30, 2025 22:38
@Koc

Koc commented Jul 30, 2025

Copy link
Copy Markdown
Contributor Author

@Aveyron-RetD I've updated PR with your suggestion. This approach requires much more changes 😁 . You can observe behavior on the video

@Aveyron-RetD

Aveyron-RetD commented Jul 31, 2025

Copy link
Copy Markdown

\o/ thanks for all your contributions. Hope they all get merged soon.

@Koc
Koc force-pushed the feature/readonly-columns-v2 branch from e035173 to f80d3b4 Compare July 31, 2025 08:03
blizzz
blizzz reviewed Jul 31, 2025
View reviewed changes

@blizzz blizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good from backend side, no blockers, 👍

@JuliaKirschenheuter @enjeck can we have a look on the frontend bits, too?

Comment thread lib/Controller/Api1Controller.php
Comment thread lib/Service/ValueObject/ViewColumnInformation.php Outdated
Comment thread lib/Db/View.php
AIlkiv
AIlkiv reviewed Aug 2, 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 the PR. I have a few minor comments.

Comment thread lib/Db/Column.php Outdated
Comment thread lib/Service/ValueObject/ViewColumnInformation.php Outdated
Comment thread src/modules/modals/EditRow.vue
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch 12 times, most recently from 3980bb6 to 1cbd076 Compare August 4, 2025 17:24
enjeck
enjeck reviewed Aug 6, 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.

Thanks! As a follow up task, let's also make sure the read-only values are not editable with inline editing that's currently being added: #364. A few column types already merged as of yesterday: #1719

@enjeck enjeck mentioned this pull request Aug 6, 2025
Closed
@enjeck

enjeck commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Thanks! As a follow up task, let's also make sure the read-only values are not editable with inline editing that's currently being added: #364. A few column types already merged as of yesterday: #1719

New issue at #1980

enjeck
enjeck reviewed Aug 6, 2025
View reviewed changes
Comment thread cypress/e2e/view.cy.js Outdated
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch 2 times, most recently from d90df48 to a7420c5 Compare August 15, 2025 14:55
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch 3 times, most recently from bcc2aee to 203c23d Compare August 18, 2025 22:02
marcoambrosini
marcoambrosini approved these changes Aug 21, 2025
View reviewed changes

@marcoambrosini marcoambrosini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very nice @Koc
The only comment I have for now is I would separate the string into "Read only" in the actions menu

@enjeck
enjeck force-pushed the feature/readonly-columns-v2 branch from 203c23d to 7a121ce Compare September 5, 2025 06:00
@Koc
Koc force-pushed the feature/readonly-columns-v2 branch from 7a121ce to 682c76f Compare September 5, 2025 09:46
@Koc

Koc commented Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

@marcoambrosini I've updated text string

image

Cypress test are failing after one of the last rebases and I have no idea why. Can anybody help me with that, please? @enjeck @blizzz

Koc added 2 commits September 7, 2025 03:57
@Koc @enjeck
Enhancement: add a read-only option to view columns
1aca11e 
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@Koc @enjeck
Enhancement: add a read-only option to view columns (code review adju…
426b46b 
…stments)

Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
@enjeck
enjeck force-pushed the feature/readonly-columns-v2 branch from 682c76f to 426b46b Compare September 7, 2025 02:57
@enjeck
fix(Cypress): use data attributes as selectors
f139c26 
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
@enjeck enjeck mentioned this pull request Sep 7, 2025
Merged
@enjeck

enjeck commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Cypress test are failing after one of the last rebases and I have no idea why. Can anybody help me with that, please? @enjeck @blizzz

I fixed a few, but commented out the readonly attribute check since I can't figure out why it's failing. Even though tests fails, manual checks show they work. I'm fine with merging as-is right now. Stylelint fixed at #2050

enjeck
enjeck approved these changes Sep 7, 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.

Thank you so much!

@enjeck
enjeck merged commit 06a33aa into main Sep 7, 2025
63 of 73 checks passed
@enjeck
enjeck deleted the feature/readonly-columns-v2 branch September 7, 2025 09:52
@enjeck

enjeck commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Thanks! As a follow up task, let's also make sure the read-only values are not editable with inline editing that's currently being added: #364. A few column types already merged as of yesterday: #1719

New issue at #1980

Done at #2049

@AIlkiv AIlkiv mentioned this pull request Nov 6, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blizzz blizzz blizzz left review comments
@AIlkiv AIlkiv AIlkiv left review comments
@marcoambrosini marcoambrosini marcoambrosini approved these changes
@enjeck enjeck enjeck approved these changes

Assignees

No one assigned

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

View feature: set permission per column: "writable" or "read-only"

6 participants

@Koc @Aveyron-RetD @enjeck @blizzz @AIlkiv @marcoambrosini