Page MenuHomePhabricator

Edit link broken on user pages
Closed, ResolvedPublic2 Estimated Story Points

Description

Story
As a logged-in user without a user page, I want the ability to create my user page in mobile by clicking the link Create a page called User:XXXXX

Description
Visit https://en.m.wikipedia.org/wiki/User:Mr_Selenium and try to create a user page.
It doesn't work sadly.

Test Plan

there should be a link to Create a page called User:Mr Selenium, but there should not be an edit pencil at the top of the page. When clicking "create a page called..." an editor overlay comes up.

  • Ensure you are logged out and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/User:Mr_Selenium and Create a page called User:Mr Selenium. When clicking the button you should see a screen which has a button "Edit without logging in"
  • Register as a new user and navigate to your user page via the link in the left menu (accessible via the hamburger icon in the top left corner) and click Create your own, which should allow you to create the page, i.e. an editor overlay comes up. Follow the editor workflow to create a user page. When you hit save the page should no longer show the "create your own button" but an edit icon should now be visible at the top of the page.
  • Navigate to https://en.m.wikipedia.beta.wmflabs.org/wiki/User:ThatDontExist193y31731 - there should be a message that makes clear there is not a user with this name and an edit pencil should appear at the top of the page.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson added a project: Regression.
Jdlrobson added a project: Web-Team-Backlog.

Seems like the router with the editor routes are missing so the #/editor/0 link does nothing. We should fix this.

MBinder_WMF set the point value for this task to 2.Sep 19 2016, 4:54 PM

Looks like this is an easy fix but I'm going to add a new browser test.

Change 312948 had a related patch set uploaded (by Jdlrobson):
Blank user pages should be editable

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

@Jdlrobson: I've left a minor question inline on PS1 of 312948.

Change 312948 merged by jenkins-bot:
Blank user pages should be editable

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

Please specify which browsers & devices to test with when you move a task to needs QA. Otherwise it defaults to all of these https://www.mediawiki.org/wiki/Reading/Web/QA_device_and_browsers_list and that may not be needed for patches like this one.

Thanks for pinging folk @Jhernandez.


I noticed that the test plan only contains the step covering what was broken. Do y'all think it'd be better if we wrote the test plan for all of the scenarios that the feature has to cover?

During code review, @bmansurov mentioned the following:

  1. One that the user own[s]
  2. One that the user doesn't own (i.e. someone else's user page)
  3. One for a username that doesn't exist

These scenarios should also be covering the user being logged-in/out.

https://en.m.wikipedia.beta.wmflabs.org/wiki/User:Mr_Selenium doesn't open up an editor overlay. If there are any recommended testing credentials let me know (a specific device, browser app, etc.) or if I have to change anything in the Settings > Apps overflow menu please let me know that too.

@Nicholas.tsg thanks for flagging. It doesn't need any additional testing credentials - It seems this needs more work.

Engineers - to replicate this locally:

Add to LocalSettings.php

$wgGroupPermissions['*']['edit'] = false;

and then view a blank user page as an anonymous user.
We should probably hide this in this case.

Change 313470 had a related patch set uploaded (by Jdlrobson):
If not possible to edit user page do not show edit link

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

Change 313690 had a related patch set uploaded (by Phuedx):
Revert "Blank user pages should be editable"

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

Added a comment and a suggest approach - @phuedx can you update the commit message or would you be okay with me doing so?

@Jdlrobson: I'm happy for you to update my commit messages whenever necessary.

From @Jdlrobson's review of rEMFR2eacd0e06c3e: Revert "Blank user pages should be editable":

The downside of reverting is we break editing on the user page again which seems more important to me then showing a misleading toast to someone who is not allowed to edit.

This is a call for @ovasileva.

Change 313862 had a related patch set uploaded (by Jdlrobson):
Protected pages should show edit link and toast when clicked

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

Change 313864 had a related patch set uploaded (by Jdlrobson):
Protected pages should show edit link and toast when clicked

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

I've submitted two patches.
To summarise the problems
Problem A: If a user goes to their user page and it is blank they cannot edit it
Problem B: If a user (anon or logged in) goes to any protected page which they cannot edit, they will not see the edit icon. Previously clicking the edit icon would show a toast telling them why the page was uneditable.

Three possible adventures for today - choose your own adventure!

  1. Merge https://gerrit.wikimedia.org/r/313862, fixes problem B. No impact on A.
  2. Merge the revert https://gerrit.wikimedia.org/r/313690: Fixes problem B but breaks problem A.
  3. Merge https://gerrit.wikimedia.org/r/313864 - A will temporarily be broken by the revert and B will be fixed. The follow up patch will fix A. Provided this is done in a timely fashion this is the same result as #1

@bmansurov, as tech lead I guess this is up to you to coordinate with @ovasileva to ensure that the right code gets cut in the branch tomorrow morning (end of day today). Feel free to ping me on IRC if you need more background.

spoke with @bmansurov - let's go with adventure 2

Change 313862 abandoned by Jdlrobson:
Protected pages should show edit link and toast when clicked

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

After speaking with @ovasileva (baha was away on IRC) it seems like the plan is to do #2 before the end of the day but strive to get #3 done too so please do review https://gerrit.wikimedia.org/r/313864 asap - I plan to swat deploy at the nearest opportunity.

Change 313690 merged by jenkins-bot:
Revert "Blank user pages should be editable"

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

Just to make sure, let's not merge the new patches until after the branch is cut.

Please take a look at https://gerrit.wikimedia.org/r/313864 regardless. It shouldn't be too scary and it would be great to have neither of these bugs. It would be great to end the day with a +/-1 on this patch at the very least.

(Also since this has revealed that the protected page tests are apparently important - I've created this follow up task T147244)

I've left some comments on the patch.

Change 313470 abandoned by Jdlrobson:
If not possible to edit user page do not show edit link

Reason:
This should have been squashed into https://gerrit.wikimedia.org/r/#/c/313864/

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

Change 313864 merged by jenkins-bot:
Blank user pages should be editable (attempt #2)

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

I moved this to "Needs More Work" because it fails step 2 of the Test Plan - "Create a page called User:Mr Selenium" does not show on the page when logged out of a user account as shown in below screencap.

Cannot edit Mr Selenium when logged out of user account.png (800×480 px, 47 KB)

@ovasileva @Nirzar is the test plan correct? Do we want to encourage anonymous editing of user pages?

Current behavior:

  1. Logged out - edit link appears and page is editable
  2. Logged in (as different user) - edit link appears and page is editable
  3. Logged in (as same user) - edit link appears and page is editable

@Jdlrobson - good point. Technically, I would like to say no, but I think it's more important that we are able to replicate desktop behavior (where anonymous and non-anonymous users can edit user page).

Thus, I think the behavior ^ is acceptable. Signing off on this.