Allow filtering by category #203

Krenair · 2017-04-05T05:27:50Z

This doesn't display anything in the queue filter GUI yet. That'll be a future commit.

Bug: T72723

The 'type' key is probably redundant here, but it's in the default config already. This doesn't display anything in the queue filter GUI. Bug: T72723

benapetr

Did you try if this works? How do you specify categories and where do you store them? I am not sure what these 3 new variables in QueueFilter are good for.

benapetr · 2017-04-05T11:43:17Z

huggle/hugglequeuefilter.cpp

@@ -75,6 +75,9 @@ HuggleQueueFilter::HuggleQueueFilter()
    this->Users = HuggleQueueFilterMatchIgnore;
    this->TalkPage = HuggleQueueFilterMatchExclude;
    this->UserSpace = HuggleQueueFilterMatchIgnore;
+    this->Type = "default";
+    this->SourceType = nullptr;


I don't really know why this compiles, there is probably an operator override, but you shouldn't assign nullptr to an object, this->Type is not a pointer, it's object. There is no need to initialize it, it's done implicitly by constructor of QString

Same for other QString

benapetr · 2017-04-05T11:45:57Z

huggle/hugglequeuefilter.hpp

@@ -113,6 +119,9 @@ namespace Huggle
            HuggleQueueFilterMatch Self;
            HuggleQueueFilterMatch UserSpace;
            HuggleQueueFilterMatch TalkPage;
+            QString Type;


What is "Type" for? Does it identify a "type" of filter? I see that so far you always use either "default" or "dynamic", if that is the case, why do you need to store this in a string? Wouldn't it be more efficient to use enumeration which is far more memory and CPU efficient?

BTW what is difference between "default" and "dynamic"?

Looked more into this I don't really understand what is point of all these 3 variables? Filter is only 1 for whole queue, so list of categories you want to exclude, or require should be defined in each filter separately. What are these 3 variables for? And how do you store a list of categories that user wants to filter now?

The variables are set in the DefaultConfig
We could allow filtering for one of (or all of) a set of categories if you want

benapetr · 2017-04-05T11:49:45Z

huggle/hugglequeuefilter.cpp

@@ -157,6 +160,13 @@ bool HuggleQueueFilter::Matches(WikiEdit *edit)
        if (this->Self == HuggleQueueFilterMatchRequire && edit->User->Username.toLower() == Configuration::HuggleConfiguration->SystemConfig_Username.toLower())
            return false;
    }
+    if (this->Type == "dynamic" && this->SourceType == "category")
+    {
+        if (edit->Status == StatusPostProcessed && !edit->Page->GetCategories().contains(QString("Category:") + this->Source))


Does this work on wikis other than English? I think that mediawiki localizes the page name, so "Category:" would probably be substituted. In that case you can use help of WikiSite object which contains a list of namespaces (NamespaceList), each namespace in huggle contains canonical name which can be retrieved using "GetCanonicalName()"

benapetr · 2017-04-05T11:51:38Z

huggle/wikiedit.cpp

+    {
+        if (this->qCategories->IsFailed())
+        {
+            Syslog::HuggleLogs->ErrorLog("Unable to fetch categories for page " + this->Page->PageName + ": " + this->qCategories->GetFailureReason());


This string is error for user, so it probably should be made localizable (just replace it with something like _l("category-fetch-fail", this->Page->PageName, this->qCategories->GetFailureReason()) and add that string to Localization/en.xml)

I think this was just following the existing pattern for other post-process queries

Yes they all need to be made translatable, there is a GCI task for that

So I'll leave this as is and it can be followed up with the GCI task?

I'm still learning C++

Don't hardcode the category namespace name to prepend to the filter source, instead, strip it from the categories of the page.

Krenair · 2017-04-05T22:22:18Z

I have tested this with the default "Candidates for speedy deletion" filter. I created my own in https://en.wikipedia.org/w/index.php?title=User%3AKrenair%2Fhuggle3.css&type=revision&diff=773914330&oldid=773805466

benapetr · 2017-04-06T08:15:21Z

OK,

But I think of a little different design here, basically right now we can have only 1 filter active in a moment. The way how you implemented this, it's not possible to filter multiple categories in each filter. Imagine someone wanted to create a filter that ignores talk pages, bots and registered users and wants to see only pages in category "United States" which also are not in category "Presidents" and "Politicians". This is not possible with your design.

So what I propose (I still don't understand what is meaning of these 3 variables you created, but whatever) - add this filter in same way as tag filter was implemented. Add 2 string lists "IgnoreCats" and "RequireCats" and let user define a list of categories for each (separated either by comma or newline, I don't know what is better).

Then the filter would just look if wikiedit has all "RequireCats" and doesn't have any of "IgnoreCats".

I hope it's clear what I mean :)

benapetr · 2017-04-06T08:17:49Z

And in your configuration file huggle3.css it would look like this:

queues:
    User defined queue #9:
        filter-ignored:exclude
        filter-bots:exclude
        filter-assisted:exclude
        filter-ip:ignore
        filter-minor:ignore
        filter-new-pages:ignore
        filter-me:exclude
        filter-users:ignore
        nsfilter-user:ignore
        filter-talk:exclude
        ignored-tags:mobileaedit,asd
        required-tags:
        required-cats:United States
        ignored-cats:Politicians, Presidents

I omitted these 3 variables you added because I just don't understand their purpose. However this is I believe what users on feedback page wanted to have

Krenair · 2017-04-06T21:56:50Z

Sure, I was just trying to stick to the existing config. I'll change it around to work like that.

…categories keys

Krenair · 2017-04-06T22:58:23Z

By the way, why does a lot of the default config appear to not be used?
https://phabricator.wikimedia.org/diffusion/GHUG/browse/master/huggle/Resources/DefaultConfig.txt$486,488-489,491-494,499,500,512,516,519-521

benapetr · 2017-04-07T06:32:01Z

Because huggle3 is a rewrite of older version of huggle. Some of these were not implemented yet.

benapetr · 2017-04-07T07:00:17Z

Some of these we probably don't want to implement. I will have a look how categories worked before, but if we have a chance to do it better way this time, I think we should use it :))

benapetr

Looks good, are you going to implement UI as well?

Krenair · 2017-04-07T14:19:59Z

@benapetr: I don't think the GUI commit got merged?

Krenair · 2017-04-07T14:43:45Z

nevermind, it got merged, had some issues with git. the way github merges commits together is a bit strange

Krenair added 2 commits April 5, 2017 06:24

Attempt to support existing config for filtering by category.

231cfc5

The 'type' key is probably redundant here, but it's in the default config already. This doesn't display anything in the queue filter GUI. Bug: T72723

include QStringList in WikiPage

ff74080

benapetr self-requested a review April 5, 2017 11:40

benapetr reviewed Apr 5, 2017

View reviewed changes

Krenair added 2 commits April 5, 2017 23:10

Don't try to set objects to nullptr

0200feb

I'm still learning C++

Fix category filtering for non-English wikis

daa4aa9

Don't hardcode the category namespace name to prepend to the filter source, instead, strip it from the categories of the page.

Change category filtering options to required-categories and ignored-…

b645a94

…categories keys

Krenair changed the title ~~Attempt to support existing config for filtering by category.~~ Allow filtering by category Apr 6, 2017

benapetr approved these changes Apr 7, 2017

View reviewed changes

Add GUI for category filtering

29d0dd2

benapetr merged commit c8c8fe3 into huggle:master Apr 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow filtering by category #203

Allow filtering by category #203

Krenair commented Apr 5, 2017

benapetr left a comment

benapetr Apr 5, 2017

benapetr Apr 5, 2017

benapetr Apr 5, 2017

benapetr Apr 5, 2017

benapetr Apr 5, 2017

Krenair Apr 5, 2017

benapetr Apr 5, 2017

benapetr Apr 5, 2017

Krenair Apr 5, 2017

benapetr Apr 6, 2017

Krenair Apr 6, 2017

Krenair commented Apr 5, 2017

benapetr commented Apr 6, 2017

benapetr commented Apr 6, 2017

Krenair commented Apr 6, 2017

Krenair commented Apr 6, 2017

benapetr commented Apr 7, 2017

benapetr commented Apr 7, 2017

benapetr left a comment

Krenair commented Apr 7, 2017

Krenair commented Apr 7, 2017

Allow filtering by category #203

Allow filtering by category #203

Conversation

Krenair commented Apr 5, 2017

benapetr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Krenair commented Apr 5, 2017

benapetr commented Apr 6, 2017

benapetr commented Apr 6, 2017

Krenair commented Apr 6, 2017

Krenair commented Apr 6, 2017

benapetr commented Apr 7, 2017

benapetr commented Apr 7, 2017

benapetr left a comment

Choose a reason for hiding this comment

Krenair commented Apr 7, 2017

Krenair commented Apr 7, 2017