Filter KNewStuff Plasma Themes by Download Tag
ClosedPublic

Authored by leinir on May 31 2019, 10:18 AM.

Details

Summary

Following a lengthy discussion on the topic[1], the first step
towards getting themes cleaned up and hidden when they don't
work is to get the ones hidden which are only for Plasma 4.
We do this by filtering out any theme which is tagged as
being for the major version 4 of plasma. We also allow themes
through which have been tested to work on both, by accepting
themes which are tagged as both 4 and 5.

[1] https://phabricator.kde.org/T8126#184198 and down a few pages

Test Plan

See themes tagged as plasma##majorversion=4 being hidden, except when they
are also tagged as plasma##majorversion=5. This can most easily be done
by launching the plasma theme dialog and opening Get New Themes from there
and turning on debugging for KNewStuff(Core) like so:

QT_LOGGING_RULES="org.kde.knewstuff*=true" kcmshell5 kcm_desktoptheme

and then watching the output, and seeing that content is being rejected
by the InequalityFilter for plasma##majorversion when appropriate.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
leinir created this revision.May 31 2019, 10:18 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 31 2019, 10:18 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.May 31 2019, 10:18 AM
leinir edited the test plan for this revision. (Show Details)May 31 2019, 10:20 AM
leinir edited the test plan for this revision. (Show Details)
leinir added a reviewer: Plasma.
leinir updated this revision to Diff 58937.May 31 2019, 11:23 AM
  • Add TagFilter (for top level tags), and don't explicitly reject 4

Is the content actually tagged appropriately yet? When I test this out I see no effect, i.e. old Plasma 4 themes like OLED Orange (https://store.kde.org/p/998790/) are still visible.

Is the content actually tagged appropriately yet? When I test this out I see no effect, i.e. old Plasma 4 themes like OLED Orange (https://store.kde.org/p/998790/) are still visible.

Not all of it, no - we do have a bit of a chicken/egg situation going on here, but at least the one you linked to isn't tagged with appropriate versioning information (https://api.kde-look.org/ocs/v1/content/data/998790 is the full xml data for that item). Fair bunch of them are, though.

Can you update the test plan to provide examples of themes that are correctly tagged so we can test this?

  • Plasma 5 only theme
  • Plasma 4 only theme
  • Theme that works on both Plasma 4 and 5

Can you update the test plan to provide examples of themes that are correctly tagged so we can test this?

  • Plasma 5 only theme
  • Plasma 4 only theme
  • Theme that works on both Plasma 4 and 5

When i first looked, for some reason i couldn't find any that matched the last option (i feel fairly certain that it was there when i originally checked), and then got sidetracked. Here is a list of three themes which match the three conditions:

Plasma 5 only theme: "OxyLight 5 Plasma theme" (id: 998609)
Plasma 4 only theme: "A Pimp Named Slickback (for the contest)" (id: 998596)
Theme which says it works on both: "OxygenDymAero7" (id: 1305103)

(and, of course, any theme which has no version set will be accepted, or we'd be filtering out the vast majority of themes on there...)

ngraham accepted this revision.Sun, Jun 30, 8:22 PM

Yep, seems to work and makes sense. Let's get this in so we can start tagging things.

This revision is now accepted and ready to land.Sun, Jun 30, 8:22 PM
leinir added a comment.Mon, Jul 1, 7:45 AM

Yep, seems to work and makes sense. Let's get this in so we can start tagging things.

Aaaawesomesauce :) Land on master, or 5.16, or?

leinir updated this revision to Diff 61120.Thu, Jul 4, 10:07 AM
  • Also filter on the ghns_excluded tag, otherwise they'll be shown...
  • Also filter on the ghns_excluded tag, otherwise they'll be shown...

I thought that was taken care of on the server side?

Yep, seems to work and makes sense. Let's get this in so we can start tagging things.

Aaaawesomesauce :) Land on master, or 5.16, or?

If the requisite support is entirely server-side, I'd even say the Plasma/5.12 branch and merge forward. Otherwise Plasma/5.16 and merge forward.

leinir added a comment.Thu, Jul 4, 2:46 PM
  • Also filter on the ghns_excluded tag, otherwise they'll be shown...

I thought that was taken care of on the server side?

No, all filtering happens client-side

leinir added a comment.Thu, Jul 4, 2:47 PM

Yep, seems to work and makes sense. Let's get this in so we can start tagging things.

Aaaawesomesauce :) Land on master, or 5.16, or?

If the requisite support is entirely server-side, I'd even say the Plasma/5.12 branch and merge forward. Otherwise Plasma/5.16 and merge forward.

Right, Plasma/5.16 it is :) Technically it /could/ go into 5.12... It has no effect unless someone runs it against Attica/KNewStuff 5.51, but i don't expect there'll be many using that particular combination ;)

This revision was automatically updated to reflect the committed changes.