Page MenuHomePhabricator

OAuth extension should register currently active change tags
Closed, ResolvedPublic

Description

It looks like the OAuth extension applies change tags, but it doesn't register the ones which are currently in use by approved consumers. As a result, a page like https://commons.wikimedia.org/wiki/Special:Tags will show all OAuth related tags as "inactive". It would actually be useful to be able to distinguish between tags set by consumers which are approved (active) and those which are not.


Version: unspecified
Severity: enhancement

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:41 AM
bzimport set Reference to bz58312.
bzimport added a subscriber: Unknown Object (MLST).

I'm not sure that the change in definition for ChangeTags::listDefinedTags() made in Gerrit change 81830 (after change tags were added to OAuth) really makes sense.

According to the comment on that function (which wasn't updated to match the changed definition), the purpose of that list is to list tags that should show up on Special:Tags even when they aren't attached to anything. If a tag shouldn't show up when it's not attached to anything (as may be the case for OAuth's tags if the consumer doesn't take any logged actions), then it shouldn't be returned by that function.

Gerrit change 81830 changed that definition, now there is no straightforward way to say "active" without also forcing it to show up in the list when there are 0 tagged changes. The non-straightforward way is to query if there are any tagged changes for each tag before adding it to the list of "defined" tags.

That change doesn't even touch ChangeTags, it only touches SpecialTags.
Did you mean some other one? Or am I misunderstanding you entirely?

(In reply to comment #2)

Or am I misunderstanding you entirely?

Yes, you changed Special:Tags to use ChangeTags::listDefinedTags() in a manner inconsistent with its former meaning.

How so? I honestly don't see any inconsistency. The way I see it here, OAuth should just use the 'ListDefinedTags' hook to define its tags, like every other extension out there.

Hey Brad, any update on this? Marking OAuth apps incorrectly as "inactive" is IMO worse than potentially having a consumer listed even if it hasn't been used to make any changes on a particular wiki.

Did we make a decision on Bartosz's change in semantics of ChangeTags::listDefinedTags(), making it effectively ChangeTags::listActiveTags() instead (and therefore making it impossible to have a defined tag that isn't "active" or an "active" tag that isn't pre-defined)?

If we're going to keep that change in semantics, then all someone has to do is add a hook function for ListDefinedTags that dumps the proper subset of possible OAuth tags into $tags. And someone needs to decide if "the proper subset" contains consumers that are still pending approval but might be being used by the creator and/or consumers that are approved but have made no logged actions.

gerritbot subscribed.

Change 187624 had a related patch set uploaded (by Anomie):
Support ListDefinedTags and ChangeTagsListActive hooks

https://gerrit.wikimedia.org/r/187624

Patch-For-Review

Change 187624 merged by jenkins-bot:
Support ListDefinedTags and ChangeTagsListActive hooks

https://gerrit.wikimedia.org/r/187624

Change 189023 had a related patch set uploaded (by Anomie):
Support ListDefinedTags and ChangeTagsListActive hooks

https://gerrit.wikimedia.org/r/189023

Patch-For-Review

Change 189023 merged by jenkins-bot:
Support ListDefinedTags and ChangeTagsListActive hooks

https://gerrit.wikimedia.org/r/189023

Anomie claimed this task.
Anomie set Security to None.