Page MenuHomePhabricator

Review and deploy the "In other projects sidebar" beta feature (part of Wikibase extension)
Closed, ResolvedPublic

Description

https://www.mediawiki.org/wiki/Wikibase/Beta_Features/Other_projects_sidebar

https://www.mediawiki.org/wiki/Extension:Wikibase_Client

Checklist

Done

  1. Create Extension: mediawiki.org page for developers and people who will install or configure the extension.
  2. Create Help:Extension: mediawiki.org page for the end user documentation. Cross-link it with the above.
  3. Request a component in [[Bugzilla]].
  4. Get the extension code in [[Gerrit]].

Not Done

  1. Request (and respond to) a product review, if applicable (''more to come here...'')
  2. Request (and respond to) a [[WMF Project Design Review Process|design review]], if applicable.
  3. Request (and respond to) a performance review
  4. Request (and respond to) a security review
  5. Show community support/desire for the extension to be deployed, if applicable.

Note: This is already enabled on FR Wikisource.


Version: wmf-deployment
Severity: normal

Details

Reference
bz66226

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 3:19 AM
bzimport set Reference to bz66226.
bzimport added a subscriber: Unknown Object (MLST).
greg created this task.Jun 5 2014, 10:59 PM

I have added it to the deployment calendar for June 24th.

(In reply to Lydia Pintscher from comment #1)

I have added it to the deployment calendar for June 24th.

I've added it to https://www.mediawiki.org/wiki/Beta_Features/New_Features
and added an infobox at https://www.mediawiki.org/wiki/Wikibase/Beta_Features/Other_projects_sidebar

@greg: Is this ready to go otherwise? (I'm guessing that it doesn't need a separate security/performance review, as it's part of the existing deployed extension?)

@Tpt or Lydia: When will this be available at beta.wmflabs? http://en.wikipedia.beta.wmflabs.org/wiki/Special:Preferences#mw-prefsection-betafeatures - It needs to be there for a week, before it can start to ride the deployment train, per https://www.mediawiki.org/wiki/Beta_Features/Package#Release_Requirements

CCing James Forrester, as Beta Features PM.

Deployment to beta is waiting on the merge of two patches by Tpt. It is hard to give a date for that atm but I will hurry it up.

While not a requirement, it would be useful for the clickthroughs on these links to be instrumented so that we can make a data informed decision about whether to promote the feature to stable later.

Also, can the preliminary security and performance review bugs be linked to this one please.

greg added a comment.Jun 17 2014, 9:40 PM

(In reply to Quiddity from comment #2)

@greg: Is this ready to go otherwise? (I'm guessing that it doesn't need a
separate security/performance review, as it's part of the existing deployed
extension?)

There is a line that needs to be drawn in the proverbial sand:

At what point does adding new features to an extension require a new security/perf review?

I don't know the answer to that, and mostly go with my gut right now (you can usually get a pretty good idea from how people talk about the features etc).

Can someone familiar with the code comment on the data flow for this? Where is it getting data from? How is it displaying it? Does it sanitize itself? How is the data modified? etc

(In reply to Jared Zimmerman (WMF) from comment #5)

Also, can the preliminary security and performance review bugs be linked to
this one please.

Agreed. Can the person who responds to my above question (Lydia? Tpt?) file the two bugs, please. Make them blockers of this bug. If they're (the reviews) easy/quick, that's even better.

(In reply to Lydia Pintscher from comment #3)

Deployment to beta is waiting on the merge of two patches by Tpt. It is hard
to give a date for that atm but I will hurry it up.

It needs to be on the beta cluster before it can go to production. We try to have it there for at least 1 week (7ish days) before.

(In reply to Greg Grossmeier from comment #6)

(In reply to Quiddity from comment #2)

@greg: Is this ready to go otherwise? (I'm guessing that it doesn't need a
separate security/performance review, as it's part of the existing deployed
extension?)

There is a line that needs to be drawn in the proverbial sand:
At what point does adding new features to an extension require a new
security/perf review?
I don't know the answer to that, and mostly go with my gut right now (you
can usually get a pretty good idea from how people talk about the features
etc).

Well, given that the actual code that generates these links was merged in Feburary (and already enabled on a WMF site), it's a bit late for a security review. The amount of code added in If8706343136ca25c0967aad3a8451283330d636f is extremely small compared to the size of the extension, and doesn't warrant a specialized review IMO.

Can someone familiar with the code comment on the data flow for this? Where
is it getting data from? How is it displaying it? Does it sanitize itself?
How is the data modified? etc

I don't understand why this is necessary?

But, it gets the data from the database, and formats it according to how the core hook wants it, which handles the display part. This is all done in Wikibase/client/includes/hooks/OtherProjectsSidebarGenerator.php.

(In reply to Jared Zimmerman (WMF) from comment #5)

Also, can the preliminary security and performance review bugs be linked to
this one please.

Agreed. Can the person who responds to my above question (Lydia? Tpt?) file
the two bugs, please. Make them blockers of this bug. If they're (the
reviews) easy/quick, that's even better.

As stated above, I don't think security/performance reviews are necessary. HTH.

greg added a comment.Jun 17 2014, 10:11 PM

(In reply to Kunal Mehta (Legoktm) from comment #7)

Well, given that the actual code that generates these links was merged in
Feburary (and already enabled on a WMF site), it's a bit late for a security
review. The amount of code added in
If8706343136ca25c0967aad3a8451283330d636f is extremely small compared to the
size of the extension, and doesn't warrant a specialized review IMO.

Saying something bypassed the process is not a reason for it not to follow the process. # of lines also isn't (inherently) an indicator of security risk.

Can someone familiar with the code comment on the data flow for this? Where
is it getting data from? How is it displaying it? Does it sanitize itself?
How is the data modified? etc

I don't understand why this is necessary?

They were questions to get an idea of how well security was thought about during the development of the feature.

greg added a comment.Jun 20 2014, 4:59 AM

Just a nit-pick (as I'm updating the calendar, getting all the links lined up):

This new beta feature should be linked on the Beta Features page on the day of. And should it live under mw.org/Wikibase/ or under mw.org/Beta_Features/ ?

<unimportant nit picks />

(In reply to Greg Grossmeier from comment #9)

This new beta feature should be linked on the Beta Features page on the day
of. And should it live under mw.org/Wikibase/ or under mw.org/Beta_Features/
?

Put it under Beta Features/ to standardize with all the others. If people get confused, a redirect under Wikibase/ could be created.

greg added a comment.Jun 20 2014, 8:49 PM

(In reply to Kunal Mehta (Legoktm) from comment #10)

(In reply to Greg Grossmeier from comment #9)

This new beta feature should be linked on the Beta Features page on the day
of. And should it live under mw.org/Wikibase/ or under mw.org/Beta_Features/
?

Put it under Beta Features/ to standardize with all the others. If people
get confused, a redirect under Wikibase/ could be created.

{{DONE}}

Is this testable on a public labs instance yet? like http://en.wikipedia.beta.wmflabs.org/

(In reply to Jared Zimmerman (WMF) from comment #12)

Is this testable on a public labs instance yet? like
http://en.wikipedia.beta.wmflabs.org/

No but you can try out the non-beta-feature version at https://fr.wikisource.org/wiki/Britannicus for example.