Page MenuHomePhabricator

automatic gardening category for pages with erroneous cargo query
Open, Needs TriagePublic

Description

It would be useful, if cargo would assign a gardening category (e.g. "Pages with incorrect cargo queries") to a thusly affected page automatically.

I create cargo queries with templates (I assume, like most people). My data structure has grown quite complex and if I change something in a table definition, that can have repercussions and ripple effects throughout my project. Identifying all affected pages and especially those, which stopped working, is an arduous task. The proposed category would help a lot. :)

Event Timeline

Oetterer triaged this task as Medium priority.Apr 26 2016, 9:46 AM
Oetterer raised the priority of this task from Medium to Needs Triage.

That's a clever idea - I never thought about it. I'll try to look into this.

@Oetterer - I looked into this a little; it might be trickier than it seems.

It's fairly easy to get a page to be added to an "errors" category if that page is saved; or if any template that is called on that page is saved. However, if it's an unrelated change that causes the error - like a change to some field name in a separate template - then it becomes more difficult, because it means adding to a category to a page simply when the page is viewed.

I'm guessing that this is what you would need, though?

Out of curiosity, just how complex is your data structure - how many Cargo tables do you have?

Yeah you are right. The page would have to be put into that category without being saved. Thinking back I got the inspiration from the Scribunto Extension, which uses exactly the mechanic you described. Unfortunate but no deal breaker for using Cargo. :) Thank you very much for your time and effort you are putting into this!

Regarding your question: I'm approaching 20 tables. But they are faily interconnected with many X-joins. Recenty I changed one field in a table to a list field and that had quite the repercussions, so the aforementioned category came to mind.

Again: Thanks for your amazing work!

While working on an other extension I stumbled over $parser->addTrackingCategory('msg-name-for-tracking-category'); (see MediaWiki Class Reference)

Would you consider adding this to parserFunctions/CargoQuery.php?

try {
  $sqlQuery = CargoSQLQuery::newFromValues( $tablesStr, $fieldsStr, $whereStr, $joinOnStr,
    $groupByStr, $havingStr, $orderByStr, $limitStr );
  } catch ( Exception $e ) {
    $parser->addTrackingCategory('msg-name-for-tracking-category');
    return CargoUtils::formatError( $e->getMessage() );
  }

You would also have to add a new message 'msg-name-for-tracking-category' to the localisation files for the tracking category. Of course with a more suitable name.

As I see it, this would place the page in the tracking category, as soon as it is rendered again. Might not immediately place it in that category after the datastructure change, but for people running jobscripts to rebuildLinks or something like that, this could do. At least it would give a little indication.

Ah, great. Have you tried that code addition, and did it work for you?

I created a faulty cargo query and the page was correctly sorted in that tracking category on preview and on save (I "accessed" a non existant table for the test). I did not - however - change my database structure and waited what would happen to that page. Still I'm assuming that on next cache refresh the above code would place it correctly in the tracking category.

Well, I just tried this, including changing the data structure to turn a functioning query in a page into a non-functioning query. The good news is that, when you do that, the "Pages with incorrect Cargo query" tag shows up at the bottom of the now-bad page. The bad news is that the page doesn't actually get added to the category, even if you refresh the cache in both places. Which is not too surprising - I'm pretty sure you need to resave a page, or call refreshLinks.php or something, to change the stored set of categories for that page.

Given that, do you still think this code addition would be a good idea? It might cause confusion...

Darn. I really hoped that refreshing the cache would sufffice.

Regarding your last question: I'm conflicted. I see your point, that this might cause confusion. Given that I am one of the admins regularly running rebuildAll (and thus refreshLinks) I - personally - would benefit from this.

However the normal expected behaviour is that if a parser functions puts a page in category, it should appear in that category. Everything else is intransparent for the normal user. So unless you also add a global switch that lets admins decide, whether to use this gardening category (whith the default being "no"), I wouldn't recommend using this patch. Sadly.

Thank you for all your effort and the amazing work!

Oh well! Yes, I agree that the category thing would be too awkward to use. Hopefully there can be another solution, though.