WIP Saying good bye to *_where_sql() functions #11407
Conversation
| foreach ($where as $w) { | ||
| $sql .= " $w and "; | ||
| } | ||
| $options = [ |
There was a problem hiding this comment.
Need a test here. Should we move this to the blog plugin? I have never used it.
| public function buildEntityClauses($qb) { | ||
| $ands = []; | ||
|
|
||
| $qb->joinEntitiesTable('rv', 'subject_guid', 'inner', 'se'); |
There was a problem hiding this comment.
Check if river API allows non-users to be subjects. If not, then we can remove this
engine/lib/tags.php
Outdated
|
|
||
| return get_data($query); | ||
| return \Elgg\Database\Metadata::find($options); |
There was a problem hiding this comment.
Go back to get_data
hypeJunction
changed the title
|
let me know when this is ready for review |
|
Not yet, more stuff coming. Hope to get it out of my life later today or tomorrow. Need to get back to real world problems |
|
I think I am done here. Should have split this up into multiple PRs, but it was like a roller coaster. I have added some performance improvements to metadata and annotations, as I have noticed there were a ton of useless queries happening during testing. Notably, metadata cache now cacheds IDs, so we don't have to read them from the database every time metadata is updated or created. |
|
This also halves the build time. I simplified unit test entity table mock and stoped querying for active plugins if boot cache is populated |
|
Could someone wipe the caches on Travis. I think we are running out of disk space. |
done... why did you think we ran out of it btw? |
|
during one of the builds it failed on multiple boxes with memory error |
| @@ -203,6 +206,7 @@ Deprecated APIs | |||
| * ``elgg_list_entities_from_relationship``: Use ``elgg_list_entities()`` | |||
| * ``elgg_list_entities_from_private_settings``: Use ``elgg_list_entities()`` | |||
| * ``elgg_list_entities_from_access_id``: Use ``elgg_list_entities()`` | |||
| * ``elgg_batch_delete_callback`` | |||
There was a problem hiding this comment.
why remove this function? It can be helpful sometimes
There was a problem hiding this comment.
I think it creates a anti-pattern. It makes it harder to read the code when used with ElggBatch. I'd rather we write explicit code with object instances that are type hinted.
|
You may want to review and merge this before it goes out of sync. I don't have any more time to invest into this. |
Rewrites elgg_get_river(), elgg_get_tags(), search hooks and other related functions with QueryBuilder. Optimizes metadata and annotation CRUD operations to reduce the number of DB queries.
Moves view rendering tests to static analysis, as they do not require a database connection
| @@ -74,6 +92,7 @@ public function assertViewOutput($expected, $view, $view_vars = [], $viewtype = | |||
| */ | |||
| public function testCanRenderViewWithEmptyVars($view, $viewtype) { | |||
| if (!elgg_view_exists($view, $viewtype)) { | |||
| dump($view); | |||
There was a problem hiding this comment.
is this left over debug code?
There was a problem hiding this comment.
if you could remove that, i'll merge this
Needs #11403
Riverrepository testselgg_get_tags()testsI am not going to bother with search tests on this. I am going to revive #10688 and add tests there.