Skip to content

Conversation

@SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Mar 26, 2025

Summary

Modified UI logic to only make one request for all author names, instead of one request per author and re-requesting the same author name for multiple versions for the same author.

Checklist

@SebastianKrupinski SebastianKrupinski self-assigned this Mar 26, 2025
@SebastianKrupinski SebastianKrupinski requested a review from a team as a code owner March 26, 2025 11:27
@SebastianKrupinski SebastianKrupinski requested review from artonge and sorbaugh and removed request for a team March 26, 2025 11:27
@artonge artonge added this to the Nextcloud 32 milestone Mar 26, 2025
@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch 2 times, most recently from 244e6e3 to 8fa16c6 Compare March 26, 2025 11:59
@SebastianKrupinski
Copy link
Contributor Author

/backport to stable31

@SebastianKrupinski
Copy link
Contributor Author

/backport to stable30

@SebastianKrupinski
Copy link
Contributor Author

/backport to stable29

@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch from 8fa16c6 to e5e0afc Compare March 26, 2025 14:46
@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch from e5e0afc to 4c6718f Compare June 3, 2025 13:27
@SebastianKrupinski
Copy link
Contributor Author

/compile

@SebastianKrupinski
Copy link
Contributor Author

@susnux can I get a second review?

@nextcloud-command nextcloud-command requested a review from a team as a code owner June 3, 2025 13:55
This was referenced Aug 22, 2025
@AndyScherzinger
Copy link
Member

/compile rebase

@nextcloud-command nextcloud-command force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch from 4dc8bc4 to 9e0ef13 Compare August 27, 2025 18:05
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Just a tiny perf suggestion.

.filter(({ mime }) => mime !== '')
.map(version => formatVersion(version, fileInfo))

const authors = await axios.post(generateUrl('/displaynames'), { users: [...versions.map(version => version.author)] })
Copy link
Member

Choose a reason for hiding this comment

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

Please make the list of requested authors unique, the backend doesn't do it automatically and thus would query the same user multiple times. You could also do this on the backend.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should work:

Suggested change
const authors = await axios.post(generateUrl('/displaynames'), { users: [...versions.map(version => version.author)] })
const authorIds = new Set(versions.map(version => version.author))
const authors = await axios.post(generateUrl('/displaynames'), { users: [...authorIds] })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@nextcloud-bot nextcloud-bot mentioned this pull request Aug 28, 2025
Comment on lines 55 to 60
versions.forEach(version => {
const author = authors.data.users[version.author]
if (author) {
version.authorName = author
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
versions.forEach(version => {
const author = authors.data.users[version.author]
if (author) {
version.authorName = author
}
})
for(const version of versions) {
const author = authors.data.users[version.author]
if (author) {
version.authorName = author
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Looks good, but @provokateurin comment still makes sense!

@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch from 9e0ef13 to 87bec0e Compare August 28, 2025 12:33
@SebastianKrupinski
Copy link
Contributor Author

/compile rebase

Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
@SebastianKrupinski SebastianKrupinski force-pushed the fix/noid-retrieve-all-authors-at-the-same-time branch from 87bec0e to 5b02d11 Compare August 28, 2025 13:04
@SebastianKrupinski
Copy link
Contributor Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@skjnldsv
Copy link
Member

Slight issue fixed in #54728

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants