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.

Sign up for GitHub

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

Jump to bottom

refactor(cron): Use IAppConfig for cron settings and migrate ajax cron away from jQuery #46009

Merged
merged 3 commits into from
Jun 29, 2024
Merged

refactor(cron): Use IAppConfig for cron settings and migrate ajax cron away from jQuery #46009

merged 3 commits into from
Jun 29, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jun 20, 2024

Summary

This contains two refactor commits for background job / cron settings:

  1. Migrate Ajax cron script to not use jQuery
  2. Use IAppConfig for setting of cron type (ajax, web-cron, cron)

Checklist

@susnux susnux added 3. to review Waiting for reviews ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 20, 2024
@susnux susnux added this to the Nextcloud 30 milestone Jun 20, 2024
@susnux susnux requested a review from blizzz June 20, 2024 12:49
@kesselb
Copy link
Contributor

kesselb commented Jun 20, 2024

Cool, Thank you 👍

come-nc
come-nc reviewed Jun 20, 2024
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

You need to adapt AccessFactory to Access constructor changes.

apps/user_ldap/lib/Access.php Outdated Show resolved Hide resolved
apps/user_ldap/tests/AccessTest.php Outdated Show resolved Hide resolved
core/Listener/BeforeTemplateRenderedListener.php Outdated Show resolved Hide resolved
@susnux susnux requested a review from come-nc June 20, 2024 16:59
@susnux
Copy link
Contributor Author

susnux commented Jun 24, 2024

/compile

@come-nc

This comment was marked as resolved.

Sign in to view
@susnux susnux force-pushed the refactor/ajax-cron branch 2 times, most recently from b225d62 to 18b90be Compare June 24, 2024 10:49
@blizzz
Copy link
Member

blizzz commented Jun 27, 2024

unit tests are still failing

@kesselb
Copy link
Contributor

kesselb commented Jun 28, 2024

279c7e2 is partly a fix for #45083.

@kesselb kesselb mentioned this pull request Jun 28, 2024
Merged
6 tasks
@susnux
refactor(core): Make AJAX cron script work without jQuery
4d6a21a 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux requested a review from kesselb June 28, 2024 15:30
@kesselb
Copy link
Contributor

kesselb commented Jun 28, 2024

I think it will be easier to manually backport the changes to the base command for 29 and thus removed the backport request.

kesselb
kesselb approved these changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

  • Login
  • occ commands
  • core-ajax-cron.js only loaded for ajax

@kesselb

This comment was marked as resolved.

Sign in to view
@susnux
refactor: Use IAppConfig for setting cron type
1477881 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
chore: Compile assets
4f79076 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux enabled auto-merge June 28, 2024 16:00
@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 28, 2024
@susnux susnux merged commit 682faad into master Jun 29, 2024
165 checks passed
@susnux susnux deleted the refactor/ajax-cron branch June 29, 2024 14:29
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icewind1991 icewind1991 icewind1991 approved these changes

@kesselb kesselb kesselb approved these changes

@Altahrim Altahrim Awaiting requested review from Altahrim

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@blizzz blizzz Awaiting requested review from blizzz

@come-nc come-nc Awaiting requested review from come-nc

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.
5 participants
@susnux @kesselb @come-nc @blizzz @icewind1991