pulsejet
feat: Add username label on shared photos
Fixes #955
I find this feature really lacking when sharing large albums and/or timelines with multiple people, so I had a shot at implementing this the other day. Currently looks as follows:
The nextcloud username is shown in the bottom left of the picture frame for all pictures that are not owned by the logged-in user, both in the Timeline and Albums view.
I had to extend the database scheme with an additional "owner" column that stores the owner UUID for every memories-indexed photo. This would probably require a lib/Migration/ handler to amend existing rows; not sure how this is normally handled -- @pulsejet ?
Possible extensions include:
- displaying full user info (ie not just the nextcloud username) in the "metadata" pane
- filtered view for a dedicated "Shared with you" menu item cf #787 -- (one of the only features that Photos has that Memories doesn't..)
This is very nice. Populating the initial data should be easy, just need a loop to copy over during a migration. I'm not aware of any cases where file ownership can transfer so that should also be ok.
The only thing remaining is to make sure we show the display name of the user (not the username). This can be done with a client side cache (per-session)
Btw it might be a while before I can get to merging this. I'm not allowed to work on this project for a bit more time.
Great, thanks! No worries about having this in the queue for a while before you can get to it :)
When you get to it, feel free to let me know if there is something I can help with from my side to get this merged
This is very nice. Populating the initial data should be easy, just need a loop to copy over during a migration. I'm not aware of any cases where file ownership can transfer so that should also be ok.
Update (force-pushed in the above commit): I added a simple Migration handler to do this. It gets the UID from the oc_storages table, based on the storage ID found in the oc_filecache table for the respective file referenced in oc_memories.
I tested this on my end with Postgres and it ran very fast on a large DB. However, not sure about the exact syntax for SQLite and MySQL. Also not sure about edge cases like External Storage in Nextcloud (right now the queries rely on the home::uid syntax in oc_storages. FWIW, I made the new UID column notnulll=false in case some of these edge cases wouldn't work.
The only thing remaining is to make sure we show the display name of the user (not the username). This can be done with a client side cache (per-session)
This would still have to be implemented. Not sure how best to go about it, but maybe an initial query that loads all unique uids from oc_memories and then translates the displayname from oc_users and send it back to the client could work(?)
The only thing remaining is to make sure we show the display name of the user (not the username). This can be done with a client side cache (per-session)
This is now also implemented (as per the force-pushed latest commit above) using a new remote UidController API for the translation and local utils/cache to speed this up for repeated translations. This was new to me, so please double check :)
displaying full user info (ie not just the nextcloud username) in the "metadata" pane
is also implemented with a new "Shared By" view when uid != current_user
@pulsejet Marking this as "ready for review" as this PR should now be feature-complete and works stable on my end (both on a postgresql and a mysql setup).
A slightly different implementation is merged (you're co-author!) in https://github.com/pulsejet/memories/commit/eb7fa255b8fb6eb9a774d1b700ff1f5b05929841
Storing the uid in the memories table (and especially the migration) seems somewhat risky. Instead we can directly query the storag table to extract the UID during the "day" query phase.
Awesome, thanks for merging this upstream! Avoiding the UID storage via storage_id is indeed neater :)