Skip to content

Comments

fix(share): Set expiration time to end of day (23:59:59)#58128

Open
nfebe wants to merge 2 commits intomasterfrom
fix/share-expiration-date-time
Open

fix(share): Set expiration time to end of day (23:59:59)#58128
nfebe wants to merge 2 commits intomasterfrom
fix/share-expiration-date-time

Conversation

@nfebe
Copy link
Contributor

@nfebe nfebe commented Feb 5, 2026

Shares now expire at the end of the selected day instead of the beginning, allowing access throughout the entire expiration day.

Also return actual stored time in API response instead of hardcoded 00:00:00 to prevent timezone-related display issues in the UI.

@nfebe nfebe requested a review from a team as a code owner February 5, 2026 23:43
@nfebe nfebe requested review from CarlSchwan, leftybournes, provokateurin and salmart-dev and removed request for a team February 5, 2026 23:43
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.

Code looks good, but this seems like a rather big change if you ask me. @AndyScherzinger was this discussed somewhere already?

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 9, 2026

@AndyScherzinger was this discussed somewhere already?

@provokateurin @nfebe no, this is new to me and would need to be discussed. Also looping in @jancborchardt since with current behavior in mind this would be unexpected for people using the feature and understanding its current behavior.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

I agree that this would change it to a more expected behavior.

But either way, there needs to be a subline to note that expiration day means end-of-day of your current timezone (or UTC, whatever we define).

Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

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

Approval in the behavior sense: This change in behavior is approved - I did not review the implementation itself (!)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Also approving behavior design-wise. This is more expected than our current approach.

@nfebe nfebe force-pushed the fix/share-expiration-date-time branch from e3b2a79 to dbaf5f5 Compare February 23, 2026 12:55
Shares now expire at the end of the selected day instead of the
beginning, allowing access throughout the entire expiration day.

Also return actual stored time in API response instead of hardcoded
00:00:00 to prevent timezone-related display issues in the UI.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe nfebe force-pushed the fix/share-expiration-date-time branch from dbaf5f5 to 5f91dd8 Compare February 23, 2026 23:53
@nfebe nfebe enabled auto-merge February 23, 2026 23:56
Update expected values in ManagerTest to reflect the new behavior
where share expiration dates are set to 23:59:59 instead of 00:00:00.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe nfebe force-pushed the fix/share-expiration-date-time branch from 5f91dd8 to 707bfc4 Compare February 24, 2026 00:56
@nfebe
Copy link
Contributor Author

nfebe commented Feb 24, 2026

/backport to stable33

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants