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

[QA] All activity mails lost their date, not only expiry mails #1131

Open
jnweiger opened this issue Sep 12, 2022 · 6 comments
Open

[QA] All activity mails lost their date, not only expiry mails #1131

jnweiger opened this issue Sep 12, 2022 · 6 comments

Comments

@jnweiger
Copy link
Contributor

Seen while testing 2.7.1-rc.1 with core 10.11.0-rc.1

Due to #1118 all mails lost their date info.
E.g. "XXX shared YYY with ZZZ" maybe should still expose the time? should it?

Probably a harmless loss, as the mail itself has a timestamp in the header.

@jnweiger jnweiger mentioned this issue Sep 12, 2022
42 tasks
@phil-davis
Copy link
Contributor

https://github.com/owncloud/core/blob/master/apps/files_sharing/lib/Activity.php has the various constants for what type of activity happened. It would be possible to only remove the time for the activities that are about share expiry: SUBJECT_LINK_EXPIRED SUBJECT_LINK_BY_EXPIRED SUBJECT_UNSHARED_* ... (and in some cases the "unshare" happened because the share owner explicitly deleted the share, so in that case the time is relevant. But in other cases the share expired exactly at the end of day, so the time is not really relevant).

What is the requirement?

@Deaddy
Copy link

Deaddy commented Jun 14, 2023

My question would be: Why was the timestamp removed in the first place? (the related issue is hidden in proprietary repo, so I do not know)

The only situation where it would not lead to any further information would be the aforementioned expiry event, which happens by the end of the day. But for any other event it seems to be valuable information, especially since some customers only subscribe to the the digest mail.

@Deaddy
Copy link

Deaddy commented Jul 4, 2023

Support could elucidate the original issue a bit more to us and this is how the situation seems to be:

  1. expiry timestamps are utterly useless and confusing, as it is the timestamp when the expiry is actually being enforced, i.e. the first time the cronjob is running after the expiry time or the first time someone tries to access the share after the expiry time
  2. thus timestamps were removed
  3. removal of the other, non-expiry timestamps is collateral damage

Consequently I assume there can be consensus on "timestamps should be brought back for non-expiry events".

@pako81
Copy link
Contributor

pako81 commented Jul 4, 2023

Proposed possible approach at #1181

@jnweiger
Copy link
Contributor Author

jnweiger commented Jul 4, 2023

@Deaddy exactly right. There is a lot of potential confusion with expiry timestamps. The exact time of the day, when something technically expires is pretty unreliable due to random timezone misunderstandings and/or cron jobs running late.

@GrendelOnCampus
Copy link

Still not fixed? I'm not happy about that. Stop weird discussions about different timezones. Timestamp including relating (maybe the server's) timezone and the users are getting all, what is needed. Just my 2 cents. (Or let Nextcloud fix it for you. scnr)

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

No branches or pull requests

5 participants