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: Node.move should also adjust the displayname #1018

Merged
merged 4 commits into from
Jul 18, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jul 15, 2024

The displayname is always set by Nextcloud,
so if it is not a custom one it is set to the basename. This means on move (and rename) we need to update the displayname.

This is an alternative to nextcloud/server#46474

We should consider making the displayname a top-level attribute like basename (cc @skjnldsv ?)

The displayname is always set by Nextcloud,
so if it is not a custom one it is set to the basename.
This means on move (and rename) we need to update the displayname.

This is an alternative to nextcloud/server#46474

We should consider making the displayname a top-level attribute like basename.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux added type: bug 🐛 Something isn't working 3. to review 3️⃣ Waiting for reviews labels Jul 15, 2024
@susnux susnux requested review from skjnldsv, Pytal and ShGKme July 15, 2024 17:19
Copy link

codecov bot commented Jul 15, 2024

Bundle Report

Changes will increase total bundle size by 1.75kB ⬆️

Bundle name Size Change
@nextcloud/files-esm 98.4kB 877 bytes ⬆️
@nextcloud/files-esm-cjs 99.63kB 877 bytes ⬆️

@skjnldsv
Copy link
Contributor

The displayname is always set by Nextcloud, so if it is not a custom one it is set to the basename. This means on move (and rename) we need to update the displayname.

This is an alternative to nextcloud/server#46474

We should consider making the displayname a top-level attribute like basename (cc @skjnldsv ?)

I guess so yes! 👍

@susnux
Copy link
Contributor Author

susnux commented Jul 18, 2024

@skjnldsv Added this as a follow up to keep the changes history clear (that is a feature and this is a fix):
#1019

For legacy reasons a fallback is still used to move it from the `attributes`
as otherwise this would be breaking.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
skjnldsv added 2 commits July 18, 2024 15:35
Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv skjnldsv enabled auto-merge July 18, 2024 13:35
@skjnldsv skjnldsv merged commit e97096c into main Jul 18, 2024
15 checks passed
@skjnldsv skjnldsv deleted the fix/make-move-change-displayname branch July 18, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review 3️⃣ Waiting for reviews type: bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants