🧾Add share metadata#8069
Conversation
AndyScherzinger
force-pushed
the
feat/noid/share-metadata
branch
8 times, most recently
from
af1f803 to
7e3e603
Compare
AndyScherzinger
requested review from
grnd-alt and
luka-nextcloud
as code owners
| $now = time(); | ||
| $groups = []; | ||
| foreach ($rows as $row) { | ||
| $timestamp = ((int)$row['board_last_modified'] > 0) ? (int)$row['board_last_modified'] : $now; |
There was a problem hiding this comment.
idk if we should really backfill that way, does the app you're doing all that for not have an option to have an unknown value?
The values we're writing in here 99% of the time are not the real created/last_modified of the acl, so an unknown value would be better imo, or at least just the "now" value so it is clear that the value was written when the update was done.
There was a problem hiding this comment.
We can use any we would want, the share review app would else have to assume one and either that is a fully hard-coded and fixed date, no matter the share, or needs to be computed each time the shares are loaded for review. So to me, compute-once made the most sense and going with a creation/modification date of the board itself seemed most feasible. However we could also go for $now if you think that would be better. But in any care I suggest to pre-compute and migrate to a date and not have a fallback logic because we lacked dates in the past.
WDYT @grnd-alt @blizzz ? "No date" feels wrong to me, but that might be me, than we can also just leave it at 0 and take that as "unknown".
Both has its advantages, one is unknown: cause it hasn't been tracked but mid-term I find this confusing to users when there are shares with known and unknown dates.
Both works for me. I can find a way to explain the unknown in the review app of course.
There was a problem hiding this comment.
My general preference is having a one time (migration) operation over some extra business logic code that then has to be kept maintained over a long time/forever. So this would be in favor of the update step.
I guess this could run long, so there should be some progress output to avoid wild cancellation by the admin.
Having a "now" date in there seems odd in itself though and might lead to wrong assumptions. This is the more critical question imo: what does it tell users and can it be misleading? Alternative coulde setting to 01-01-1970 and still handling as "unknown", but this brings the extra treatment again.
There was a problem hiding this comment.
I'll then go for 0 as in 01-01-1970. Currently not shown in the deck UI anyways and the share review app can than have some handling for the magic number 👍
…s to deck_board_acl Adds two integer columns (default 0) to track when a share was created and last modified. A value of 0 is a magic sentinel meaning "unknown" and covers all pre-existing shares. Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Records created_at on share insertion and updates last_modified_at on every subsequent modification. The created_at value is preserved across updates. Existing shares retain the 0 sentinel (unknown). Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
AndyScherzinger
force-pushed
the
feat/noid/share-metadata
branch
from
7e3e603 to
4f1a9d7
Compare
3 participants
Summary
While starting to work on #8052 (nowhere near completion..) to implement the sdhare review capability I realized that the share date always matched 1970 and digging and asking @grnd-alt it turned out deck shares have no creation and no last-edit date, like for example files/folder or tables shares. Yet for a share review app, that also stores a "review date" such timestamps are important to show a changed share for a re-review in case it changed. Hence creation and edit dates a crucial.
TODO
Checklist
🤖 AI (if applicable)