Page MenuHomePhabricator

Curator gets minor edit count
Closed, ResolvedPublic

Description

"As a Curator, I want to see how many minor revisions have been made to a page, so that [I can judge the amount of copy-editing that has been done...?]."

This is similar to the other edit count endpoints, but for minor edits only.

GET /page/{title}/history/counts/minor

Return number of minor edits. If the number of minor edits exceeds a maximum value (currently 500), the maximum value is returned and the limit flag is set to true.

Request body: none

Notable request headers: none
Status codes:
200 - OK
403 - Not authorized to read this page
404 - no such page

Notable response headers: none

Body: JSON, object with following fields
count: total number of minor edits, or maximum allowed results
limit: boolean flag indicating that the maximum allowed results were reached

Details

Related Gerrit Patches:

Event Timeline

eprodromou updated the task description. (Show Details)Oct 15 2019, 4:08 PM
eprodromou updated the task description. (Show Details)
eprodromou renamed this task from Curator get minor edit count to Curator gets minor edit count.Oct 15 2019, 4:11 PM

Body: JSON, object with following fields
minor: total number of minor edits

Did we want to change "minor" to "count" in the JSON to match the existing behavior of the other count types?

eprodromou updated the task description. (Show Details)Oct 16 2019, 6:01 PM

Body: JSON, object with following fields
minor: total number of minor edits

Did we want to change "minor" to "count" in the JSON to match the existing behavior of the other count types?

Yes, I thought I did that yesterday!

Looking at this now in engineering task T235587: Add minor edit count to REST API history count endpoint, all the other count types explicitly include "edits":

const ALLOWED_COUNT_TYPES = [ 'anonedits', 'botedits', 'editors', 'edits', 'revertededits' ];

(Well, except "editors" which is a little different)

Unless there's strong object, I'm going to call this "minoredits" in the path. So instead of:

GET /page/{title}/history/counts/minor

I'm going to implement:

GET /page/{title}/history/counts/minoredits

eprodromou triaged this task as Medium priority.Oct 23 2019, 7:37 PM
eprodromou added a project: MediaWiki-REST-API.

@BPirkle sorry, I should have been explicit.

@Pchelolo asked in another thread that we start moving away from "*edits" to have the same flag for both the history and count endpoints. At this point we'll need to deprecate those "*edits" endpoints and make new ones without "edits". I wanted to skip that step with "minor". So, please don't add "edits" to the end of minor; the other ones are going to change to be more like "minor", not vice versa.

Acknowledged, @eprodromou

If I understand T232485: RFC: Core REST API namespace and version correctly, the deprecation process for the older parameter values is trivial from a coding standpoint. Shall we hot-add a task for deprecating the old parameters and adding the new ones to the current sprint? That way, as few callers as possible (ideally, none) ever use the old values.

@BPirkle I like that idea! I'll add tasks right now.

That way, as few callers as possible (ideally, none) ever use the old values.

We haven't set wgEnableREST anywhere except labs... so now there should be exactly 0 callers :)

In that case - can we sneak this in without deprecation and just do it?

eprodromou added a comment.EditedOct 24 2019, 7:20 PM

In that case - can we sneak this in without deprecation and just do it?

We've done a handover to iOS team. I'd like not to change things up on them. Let's do the deprecation, and I'll check with them, and if they haven't started using it or can make the change easily, we'll take out the "deprecated" strings.

Change 546197 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Rename/deprecate some REST history count types

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

Uploaded a patch that does full deprecation, with support for both old and new names. It sends "Deprecation" and "Link" headers if someone uses the old names. It wasn't 100% clear to me if T232485: RFC: Core REST API namespace and version required us to send these headers for changes in parameters, or if that was just when we deprecated entire endpoints. But I also didn't see that it prohibited sending them, and it seemed like a good idea.

I'd appreciate thoughts on if I did the headers the way we want. As this is our first official deprecation, I didn't have anything to compare to (other than trying to do what https://tools.ietf.org/html/draft-dalal-deprecation-header-00 said, and that's not exactly a code example).

If we decide we'd rather just change the names and not do deprecation, it'd be easy to rip all that out. But I suppose there's benefit in having an example of deprecation that other code can copy. So I guess I don't have strong feelings either way.

Here are the deprecation-related headers the endpoint is currently sending if a deprecated count type is specified:

Deprecation: version="v1"
Link: <https://www.mediawiki.org/wiki/API:REST/History_API#Get_page_history_counts>; rel="deprecation"

(A "deprecation" Link Relation type is available per https://tools.ietf.org/html/draft-dalal-deprecation-header-00)

Some questions, since this may become the general pattern for API deprecations:

  1. right now this just points to the endpoint documentation (which has not been updated for the deprecation, but will be). Is that what we want, or do we want a page in the documentation just for deprecations, so that developers can easily scan one page to see what changes they need to make?
  2. should we update the main RELEASE-NOTES-X.XX file for API deprecations? Would that even make sense, given that the API has its own versioning separate from mediawiki's?
  3. should the API have its own release notes file, maybe in the includes/Rest directory?
  4. is there a sensible way to put a @deprecated tag in the code? In this case, we could put it above the DEPRECATED_COUNT_TYPES definition, but I'm not sure that works as a general pattern. If we did that, would we reference an API version and not a mediawiki version?
  5. what do already we do for Action API deprecation? Does that give us any patterns to follow?
Anomie added a subscriber: Anomie.Oct 28 2019, 6:59 PM
  1. what do already we do for Action API deprecation? Does that give us any patterns to follow?

Probably not any patterns to follow, unfortunately, since the mechanism is too different. The Action API outputs its deprecations in the response body, normally without external links. Some examples:

Also I usually send email to the mediawiki-api-announce mailing list when something is deprecated and again when it is removed.

  1. right now this just points to the endpoint documentation (which has not been updated for the deprecation, but will be). Is that what we want, or do we want a page in the documentation just for deprecations, so that developers can easily scan one page to see what changes they need to make?

The intent of that draft RFC seems to be that it would link to the endpoint documentation, and the endpoint documentation should contain text about the deprecation. If I were doing something like this for the Action API, I'd probably link to the autogenerated-documentation, like this and this for the above examples.

  1. should we update the main RELEASE-NOTES-X.XX file for API deprecations? Would that even make sense, given that the API has its own versioning separate from mediawiki's?
  2. should the API have its own release notes file, maybe in the includes/Rest directory?

The RELEASE-NOTES file has a section for client-visible Action API changes ("Action API changes"). A section for client-visible changes to the REST API would probably be a good idea, people like to know what's going to break when they upgrade MediaWiki.

RELEASE-NOTES also has a section for PHP-developer-visible Action API changes ("Action API internal changes"). Possibly we should merge that into the "New developer features", "Breaking changes", and "Deprecations" sections now, since there aren't so many now as there were in say 1.25.

  1. is there a sensible way to put a @deprecated tag in the code? In this case, we could put it above the DEPRECATED_COUNT_TYPES definition, but I'm not sure that works as a general pattern. If we did that, would we reference an API version and not a mediawiki version?

Putting @deprecated on the constant would indicate that the PHP constant itself is deprecated, not that the things it names are deprecated in the external API.

eprodromou updated the task description. (Show Details)Oct 30 2019, 4:25 PM

@apaskulin now the doc page has to be updated.

Change 546197 merged by jenkins-bot:
[mediawiki/core@master] Rename/deprecate some REST history count types

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

apaskulin added a comment.EditedOct 30 2019, 6:12 PM

Updated the docs with the new filter names. I didn't include a deprecation notice in the docs because the endpoint is still fairly new, but I can go back and add one if we think it's necessary. In general, I'd like to include an API changelog in the docs, either imported from source or maintained on the wiki.

Addendum: Leaving this comment here as a followup to the previous two comments, but noting that this change applies to T236323, T236324, and T236325, not the minor edit count endpoint.

eprodromou closed this task as Resolved.Nov 5 2019, 4:10 PM