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

Add a simple admin page for background jobs #506

Conversation

eugene-manuilov
Copy link
Contributor

@eugene-manuilov eugene-manuilov commented Aug 29, 2022

Summary

Fixes #491

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz closed this Aug 29, 2022
@felixarntz felixarntz deleted the feature/background-job-taxonomy--admin-screen branch August 29, 2022 17:24
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@eugene-manuilov Leaving some early thoughts here, for reference.

I renamed this branch to experiment/* because we shouldn't use feature/* anymore except for actual feature branches.

*/

$screen = get_current_screen();
$screen->post_type = 'post';
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? Preferably the taxonomy shouldn't be attached to any specific post type.

@@ -59,7 +64,7 @@ function perflab_register_background_job_taxonomy() {
* `wp_set_object_terms` which do not check if there is relationship
* between object and taxonomy.
*/
register_taxonomy( 'background_job', array(), $args );
register_taxonomy( 'background_job', array( 'post' ), $args );
Copy link
Member

Choose a reason for hiding this comment

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

See my other comment. This feels a bit like a hack? It shouldn't be tied to the post post type.

Comment on lines +14 to +23
function perflab_register_background_jobs_page() {
add_management_page(
__( 'Background Jobs', 'performance-lab' ),
__( 'Background Jobs', 'performance-lab' ),
'manage_categories',
'background-jobs',
'perflab_render_background_jobs_page'
);
}
add_action( 'admin_menu', 'perflab_register_background_jobs_page' );
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if instead of a custom page we could rather use the regular edit-tags.php?taxonomy=background_job screen, even if we added the link to it in a custom place in the admin menu.

@mxbclang
Copy link
Contributor

@eugene-manuilov Reassigning to you to review Felix's initial feedback.

@mxbclang
Copy link
Contributor

@eugene-manuilov Apologies, looks like this has been closed.

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.

3 participants