WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 20 months ago

#41266 assigned enhancement

Not hard coding the table alias prefix in WP_Meta_Query would make class more extendable

Reported by: thomaslhotta Owned by: noisysocks
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Query Keywords: good-first-bug has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

Hi,

I am extending the WP_Meta_Query class for my own uses with custom tables that have the same structure as meta tables. It is an incredibly useful class for this. However the fact that the table alias prefix mt on line 535 is hard coded forces me to do awkward and complex string replaces if I want to use my extensions alongside built in meta queries.

I think changing the prefix from being hard coded to a property/function of the meta query object would make this class much more versatile for plugin developers.

Attachments (3)

41266-approach-1.diff (1.3 KB) - added by andizer 2 years ago.
41266-approach-2.diff (653 bytes) - added by andizer 2 years ago.
41266.patch (2.3 KB) - added by noisysocks 23 months ago.
A patch based on Approach 1 that includes unit tests

Download all attachments as: .zip

Change History (11)

#1 @boonebgorges
2 years ago

  • Keywords needs-patch good-first-bug added

I agree. This, and the lack of flexibility for the $meta_table name itself (in get_sql()), are long term annoyances for me. Could you suggest an approach in a patch?

#2 @andizer
2 years ago

I've added two patches both containing a different approach. The first patch works with a property having a default value being mt and a setter for overriding that value. The second patch works with a hook to override the default value.

If one of those patches will be merged, please also gives props to ireneyoast

#3 @janw.oostendorp
2 years ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

#4 follow-up: @ryotsun
2 years ago

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

#5 in reply to: ↑ 4 ; follow-up: @janw.oostendorp
2 years ago

Replying to ryotsun:

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

I don't understand what you are asking. Did I miss something?

#6 in reply to: ↑ 5 @ryotsun
2 years ago

Ahhh, I'm sorry.
I wanted to say "approach". Not "ticket".

2 patches are uploaded, so I just wanna know which one would be chosen.

Replying to janw.oostendorp:

Replying to ryotsun:

@janw.oostendorp
Hello, it's just a quick question.
Which ticket are you going to choose?

I don't understand what you are asking. Did I miss something?

@noisysocks
23 months ago

A patch based on Approach 1 that includes unit tests

#7 @noisysocks
23 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Personally, I think Approach 1 is the cleaner solution. WP_Meta_Query is a class and we may as well take advantage of an object oriented approach to solving this problem.

I've added a unit test to 41266-approach-1.diff and uploaded it as 41266.diff.

#8 @DrewAPicture
20 months ago

  • Owner set to noisysocks
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

Note: See TracTickets for help on using tickets.