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

merge last_activity and last_check updates #34056

Closed
wants to merge 1 commit into from

Conversation

icewind1991
Copy link
Member

updateActivity has been updated to also write any other changed field.

the debounce for updating last_activity is changed so it always updates if another field of the token has been updated, this ensures that last_check if updated even if last_activity is still in the debounce period.

This is pretty much a 2nd version of #1037, which was reverted(#29682) after the last_activity updating changed by #29357

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Sep 13, 2022
@icewind1991 icewind1991 added this to the Nextcloud 25 milestone Sep 13, 2022
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, skjnldsv and come-nc and removed request for a team September 13, 2022 14:56
@icewind1991 icewind1991 force-pushed the merge-token-updates branch 2 times, most recently from 7217c0c to 7d434e8 Compare September 13, 2022 17:14
@icewind1991
Copy link
Member Author

I've done some basic testing to ensure that both last_activity and last_check get updated under normal use

This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@come-nc
Copy link
Contributor

come-nc commented Mar 21, 2023

I do not have enought understanding/knowledge to approve this, sorry

@come-nc come-nc removed their request for review March 21, 2023 10:53
This was referenced May 3, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@icewind1991 icewind1991 force-pushed the merge-token-updates branch from 491f058 to 568c4b4 Compare June 21, 2024 08:55
@susnux susnux requested review from nfebe and removed request for CarlSchwan June 21, 2024 09:00
the debounce for updating last_activity is changed so it always updates if another field of the token has been updated, this ensures that last_check if updated even if last_activity is still in the debounce period.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the merge-token-updates branch from 568c4b4 to f6d18f8 Compare June 21, 2024 09:01
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 from my understanding, very clean!

@susnux
Copy link
Contributor

susnux commented Jun 21, 2024

But the test needs to be adjusted @icewind1991

 There was 1 failure:

1) Test\User\SessionTest::testLoginLastCheckUpdate
Expectation failed for method name is "updateToken" when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.

@ChristophWurst
Copy link
Member

Wasn't this addressed with #45026?

This was referenced Jul 30, 2024
@Altahrim Altahrim mentioned this pull request Aug 5, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 6, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants