#6063 closed enhancement (fixed)
Support meta query in bp_has_message_threads()
Reported by: | r-a-y | Owned by: | r-a-y |
---|---|---|---|
Milestone: | 2.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Messages | Keywords: | has-patch |
Cc: |
Description
Now that we have a meta table for messages, it would be nice to get meta queries working with bp_has_message_threads()
.
Attached patch does this.
Some notes:
- In order to support
'meta_query'
as an extra parameter, I changedBP_Messages_Thread::get_current_threads_for_user()
/BP_Message_Box_Template
to use one argument instead of multiple arguments. - I've refactored
BP_Messages_Message::get_current_threads_for_user()
so the SQL statement isn't duplicated in two spots. - Adds a unit test. If you analyze the test, there's a hint at getting a "Starred" box working. Might be better if this were a taxonomy instead of using meta, but it's just a test :)
Attachments (8)
Change History (21)
This ticket was mentioned in Slack in #buddypress by mamaduka. View the logs.
10 years ago
#3
@
10 years ago
Updated patch.
6063-assoc-array-args.2.diff includes changes for BP_Message_Box_Template
as well, to use array of arguments.
#5
@
10 years ago
6063-inner-joins.diff converts STRAIGH_JOINs to INNER JOINs for better compatibility with meta query. Tested with existing test for messages.
#7
@
10 years ago
Thanks r-a-y for getting those patches in.
r9301 has wrong ticket reference, so I'll just leave it here.
Any suggestions recommendations regarding JOIN changes? Current patch only update original big SQL string, but we can also convert it and follow other WP/BP standards and use array of SQL statements, joins and clauses. Which I think should make code more readable.
/cc @boonebgorges
#8
@
10 years ago
Yeah, I kinda botched the commit message for r9301 and also forgot to reference this ticket :(
The INNER JOIN
change looks okay. I think combined with my changes from 01.patch
, we should be good.
sql-clauses.patch
includes the INNER JOIN addition and uses an array to form the SQL statement.
meta-query.patch
includes the actual meta query additions + unit tests.
I'm very close to committing both, but I think we need more unit tests for meta query to demonstrate the JOIN
/ INNER JOIN
issue.
#9
@
10 years ago
r-a-y,
Any particular test cases in mind? I can get similar ones that we have in other place to cover different cases of meta queries.
The meta_query changes look solid to me!
I feel somewhat uneasy about all the refactoring of these methods, given the sad state of test coverage. (Are you surprised by this uneasiness? ;) ) Maybe we could at least get a test for each of the methods where you're switching to the array notation for params (with the
@expectedDeprecated
flag to ignore the_doing_it_wrong()
) just to demonstrate that the old-form params are translated properly?Otherwise yeah, let's do this for 2.2.