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

Replace strings with constants in WP_Job_Manager_Post_Types #2687

Merged
merged 29 commits into from
Dec 26, 2023
Merged

Replace strings with constants in WP_Job_Manager_Post_Types #2687

merged 29 commits into from
Dec 26, 2023

Conversation

fjorgemota
Copy link
Member

@fjorgemota fjorgemota commented Dec 18, 2023 •
edited
Loading

Fixes #2663

Changes Proposed in this Pull Request

  • Replace strings with constants in WP_Job_Manager_Post_Types

Testing Instructions

Test if the plugin is generally work. Some examples:

  1. Create a Job Type
  2. Create job categories
  3. Create job listings using the data created above
  4. Update/delete job categories, types and listings
  5. Try to uninstall the plugin
  6. etc.

Release Notes

  • [New] Add constants to refer to IDs of post types, capabilities and taxonomies
Plugin build for df7b39f
📦 Download plugin zip
▶️ Open in playground

@fjorgemota fjorgemota self-assigned this Dec 18, 2023
@fjorgemota
Use constant 'TAX_LISTING_CATEGORY' instead of string 'job_listing_ca…
7ab82b2 
…tegory'
@fjorgemota
Make jobs_by_category field reuse values for comparison
8e8d976 
The idea here is to unify the $selected_category logic and also make it use constants related to the taxonomy constant for listing categories
@fjorgemota
Add constant for job_listing_type taxonomy name as TAX_LISTING_TYPE
325d69b
@fjorgemota
Use constant PT_LISTING instead of the string 'job_listing'
d77d811
@fjorgemota
Use constant CAP_MANAGE_LISTINGS instead of string 'manage_job_listings'
86779e6
@fjorgemota
Convert references from 'job_listing' to constant PT_LISTING
5db891c
@fjorgemota
Use constant PT_GUEST_USER instead of string 'job_guest_user'
2bcc7bb
@fjorgemota
Add constants for capabilities
3a2fcae 
Just in a single commit because it is really a bunch of capabilities
@fjorgemota
Use constant CAP_EDIT_LISTING instead of string 'edit_job_listing'
f7aa7f7
@fjorgemota
Use constant CAP_READ_LISTING instead of string 'read_job_listing'
c1469d3
@fjorgemota
Use constant CAP_DELETE_LISTING instead of string 'delete_job_listing'
bb0f603
@fjorgemota
Use constant CAP_EDIT_LISTINGS instead of string 'edit_job_listings'
4eed349
@fjorgemota
Use constant CAP_EDIT_OTHERS_LISTINGS instead of string 'edit_others_…
e5307f1 
…job_listings'
@fjorgemota
Use constant CAP_PUBLISH_LISTINGS instead of string 'publish_job_list…
cd80b33 
…ings'
@fjorgemota
Use constant CAP_READ_PRIVATE_LISTINGS instead of string 'read_privat…
539b45a 
…e_job_listings'
@fjorgemota
Use constant CAP_DELETE_LISTINGS instead of string 'delete_job_listings'
f47352d
@fjorgemota
Use constant CAP_DELETE_PRIVATE_LISTINGS instead of string 'delete_pr…
a5d7e1d 
…ivate_job_listings'
@fjorgemota
Use constant CAP_DELETE_PUBLISHED_LISTINGS instead of string 'delete_…
382f5ac 
…published_job_listings'
@fjorgemota
Use constant CAP_DELETE_OTHERS_LISTINGS instead of string 'delete_oth…
b7949c0 
…ers_job_listings'
@fjorgemota
Use constant CAP_EDIT_PRIVATE_LISTINGS instead of string 'edit_privat…
a5edf76 
…e_job_listings'
@fjorgemota
Use constant CAP_EDIT_PUBLISHED_LISTINGS instead of string 'edit_publ…
26cadf6 
…ished_job_listings'
@fjorgemota
Use constant CAP_MANAGE_LISTING_TERMS instead of string 'manage_job_l…
66cf2c7 
…isting_terms'
@fjorgemota
Use constant CAP_EDIT_LISTING_TERMS instead of string 'edit_job_listi…
ea62408 
…ng_terms'
@fjorgemota
Use constant CAP_DELETE_LISTING_TERMS instead of string 'delete_job_l…
0bd9e48 
…isting_terms'
@fjorgemota
Use constant CAP_ASSIGN_LISTING_TERMS instead of string 'assign_job_l…
5326a9b 
…isting_terms'
@fjorgemota
Use constant CAP_EDIT_LISTINGS instead of string 'edit_job_listings'
8117aa5
@fjorgemota fjorgemota requested a review from a team December 22, 2023 01:48
@fjorgemota fjorgemota marked this pull request as ready for review December 22, 2023 01:48
mikeyarce
mikeyarce reviewed Dec 22, 2023
View reviewed changes
includes/admin/class-wp-job-manager-cpt.php
$selected_category = isset( $_GET['job_listing_category'] ) ? sanitize_text_field( wp_unslash( $_GET['job_listing_category'] ) ) : '';
echo "<select name='job_listing_category' id='dropdown_job_listing_category'>";
echo '<option value="" ' . selected( $selected_category, '', false ) . '>' . esc_html__( 'Select category', 'wp-job-manager' ) . '</option>';
echo "<select name='" . esc_attr( \WP_Job_Manager_Post_Types::TAX_LISTING_CATEGORY ) . "' id='dropdown_job_listing_category'>";
Copy link
Member

Choose a reason for hiding this comment

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

Do we also want to use the constant in a place like this? id='dropdown_job_listing_category'?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about doing so, but I think the main issue with that is that id is the sort of attribute that might be used from the CSS/JS side to handle the field somehow (e.g., select2 stuff, for instance).

And given that passing these constants to the JS side just to handle this looks a bit..too much, I decided just to leave it as it is. Right now, this whole "replacing magical strings with constants" looks already challenging enough (in the sense of risk of breaking things)

mikeyarce
mikeyarce approved these changes Dec 22, 2023
View reviewed changes
Copy link
Member

@mikeyarce mikeyarce left a comment

Choose a reason for hiding this comment

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

Thanks for taking on this task, @fjorgemota !
I looked through every replacement and double checked them and they all look good! I left a non-blocking question about if we want to use Constants in some class names.

@fjorgemota fjorgemota merged commit 2be6144 into trunk Dec 26, 2023
8 checks passed
@fjorgemota fjorgemota deleted the change/post-types-constants branch December 26, 2023 13:59
@yscik yscik added this to the 2.2.0 milestone Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikeyarce mikeyarce mikeyarce approved these changes

Assignees

@fjorgemota fjorgemota

Labels
None yet
Projects
None yet
Milestone
2.2.0
Development

Successfully merging this pull request may close these issues.

Replace strings with constants in class-wp-job-manager-post-types.php
3 participants
@fjorgemota @mikeyarce @yscik