Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better Block Message Support #4458

Merged
merged 19 commits into from
Feb 10, 2023
Merged

Better Block Message Support #4458

merged 19 commits into from
Feb 10, 2023

Conversation

tonisevener
Copy link
Collaborator

@tonisevener tonisevener commented Feb 2, 2023 •
edited
Loading

Phabricator:
https://phabricator.wikimedia.org/T169013
https://phabricator.wikimedia.org/T275118
https://phabricator.wikimedia.org/T201640

Notes

This PR adds improved display of blocked messages to users when creating an account, publishing an article description, and publishing an edit in our section editor. It adjusts the API calls in these areas and parses the error responses. It also adds a new blocked message panel (from our existing Panels.swift set of views) and disables the editors when the user is blocked.

This PR is best reviewed commit-by-commit. I tried to leave detail in the commit messages. Any developer testing should be tested with all commits in place.

There are a couple of things remaining after this PR for blocked messages:

  1. Showing a confirmation if the user is going to lose their edits after tapping a link in the panel, which will be integrated after Add alert when closing the editor if wikitext was modified #4454 is merged.
  2. We have some url routing issues with some of the blocked messages, like our native talk page intercepting a link like https://en.wikipedia.org/wiki/Special:MyTalk. I put up a couple of temporary fixes for EN, but a better approach needs to be in place for other language support (see Phab task).

Test Steps

  1. Block one of your accounts on Test Wikipedia (if you don"t have admin privileges, I can block one for you) and on Test Wikidata.
  2. Log into that account on device. Set up your primary app language as non-Test language (this is so that the correct MediaWiki:Blockedtext template is fetched after blocked API error. Test Wikipedia doesn"t have one set up).
  3. Visit an article on Test Wikipedia. Go to section editor. Confirm blocked panel appears after landing on editor, and editor is disabled.
  4. Go to article description editor by tapping the first pencil (you may need to hunt around for a Test article with a description already in place...there"s something on the PCS-side that doesn"t show the "Add article description" button for those without).
  5. Confirm blocked panel appears after landing on article description editor, and description editing is disabled.
  6. Temporary adjust code so that article description editor updates on Wikidata rather than on Test Wikipedia. The changes in this temp branch commit will do it.
  7. Repeat steps 4 & 5. Confirm blocked panel still appears.
  8. Brief regression testing - unblock account on Test Wikipedia and confirm you can successfully publish an edit in section editor.
  9. Now test create account blocked error. Set up app languages to only contain Test Wikipedia. Then re-block account, making sure there"s also an IP block in place.
  10. Try to create an account from our create account screen. Confirm blocked message appears after attempting to create an account.

Note: I also have blocked panels showing upon Publish, not just landing on the editors. Test steps for this will be a bit tedious - basically repeating 1-8, but unblocking your account in between each step so you can get past disabling of the editors. I didn"t include these steps in the interest of leaning on QA more for this.

Screenshots

Create Account Error
IMG_0157

Section Editor Error
IMG_0156

mazevedofs and others added 11 commits January 30, 2023 19:07
@mazevedofs @tonisevener
Add readonly option to codemirror setup
1f179bc
@tonisevener
Prepare panels for displaying blocked errors
505e5fd 
- Allow subheadingHTML to display links
- Fix safe area extended bug
- Add BlockedPanelViewController
@tonisevener
Add error models and resolving helper methods to Fetcher superclass
88bc85b 
- Determines best error from array of errors
- Parses error blockreason from wikitext to html
- Fetches blocked template if necessary and updates placeholders with blockinfo
@tonisevener
Display blocked panel upon section editor publish
d2c32e6 
- Update WikiTextSectionUploader params so that we get more error info
- Send response object into Fetcher helper method to resolve blocked error
- Pass error informaation back to EditSaveViewController
- Present panel and handle link taps
@tonisevener
Display blocked panel upon landing on section editor
a7193f2 
- Update SectionFetcher params so that we get more error info
- Send blockInfo values into Fetcher helper method to resolve blocked error
- Pass error information back into SectionEditorViewController
- Don"t configure web view until after wikitext is fetched
- Display blocked panel and set editor to read only
@tonisevener
Fix tests caused by previous commit
a2325a6
@tonisevener
Allow short description template editing through native view for Test…
3b83c73 
… Wikipedia
@tonisevener
Display blocked panel for EN article description edits
68f3ea6 
- DescriptionEditViewController - evaluate error upon publish, display blocked panel if needed
- ShortDescriptionController - fetch block information upon landing and pass back to view controller
- DescriptionEditViewController - display block panel upon landing and disable controls.
- Minor renaming for clarity
@tonisevener
Display blocked panel for non-EN article description edits
84a6e28 
- WikidataFetcher - Add method to fetch block information upon landing
- WikidataFetcher - Adjust publish parameters & codable response to capture block information
- WikidataFetcher - Resolve blocked error, pass back to view controller
- WikidataFetcher - General model reordering / cleanup
- DescriptionEditViewController - evaluate error upon publish, display blocked panel if needed
- WikidataDescriptionController - fetch block information upon landing and pass back to view controller
@tonisevener
Temporary fix for Special pages routing in some error messages
31e31cb
@tonisevener
Show blocked messages when creating account
5eadc3f 
- Adjust parameters to get html in response
- Pass back html to view controller as failure
- Present blocked panel from account creation view controller
@tonisevener tonisevener requested review from a team and staykids and removed request for a team February 2, 2023 16:15
@tonisevener tonisevener mentioned this pull request Feb 7, 2023
Merged
@staykids staykids self-assigned this Feb 7, 2023
staykids
staykids reviewed Feb 9, 2023
View reviewed changes
Copy link
Contributor

@staykids staykids left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice – definitely more involved than I initially expected!

I haven"t yet tested in use beyond confirming basic presentation of the block panel when section editing from a blocked account, but below is my first pass of notes from pure code review.

Wikipedia/assets/codemirror/codemirror-index.html Outdated Show resolved Hide resolved
Wikipedia/Code/ScrollableEducationPanelViewController.swift Outdated Show resolved Hide resolved
WMF Framework/Fetcher.swift Outdated Show resolved Hide resolved
Wikipedia/Code/SectionFetcher.swift Show resolved Hide resolved
Wikipedia/Code/SectionFetcher.swift Outdated Show resolved Hide resolved
Wikipedia/Code/WikidataFetcher.swift Outdated Show resolved Hide resolved
@staykids staykids removed their assignment Feb 9, 2023
@staykids staykids self-assigned this Feb 9, 2023
staykids
staykids approved these changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@staykids staykids left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Was able to test most edit/creation block paths after I got Admin rights on test Wiki (other than Wikidata blocks), but as mentioned perhaps best to rely on QA for exhaustive testing.

Looks like just the conflict with our shared changes (sorry!) to the section editor to resolve.

@staykids staykids removed their assignment Feb 10, 2023
@tonisevener
Copy link
Collaborator Author

@staykids thanks! Merged conflict is fixed. To merge our two features, I changed it so that if a blocked message does return from the wikitext call, we disable the automatic edit notice presentation (I feel there"s little need to show it to them if the editor is disabled). Writing up a Phab comment now for design to confirm this thinking.

It was difficult to test because I doubt very many Test Wikipedia articles have edit notices. I basically did it by mapping the edit notice response locally with a response taken from EN Wikipedia.

@staykids
Copy link
Contributor

@staykids thanks! Merged conflict is fixed. To merge our two features, I changed it so that if a blocked message does return from the wikitext call, we disable the automatic edit notice presentation (I feel there"s little need to show it to them if the editor is disabled). Writing up a Phab comment now for design to confirm this thinking.

It was difficult to test because I doubt very many Test Wikipedia articles have edit notices. I basically did it by mapping the edit notice response locally with a response taken from EN Wikipedia.

@tonisevener Cool - I tested by just stubbing out an always successful/available but empty edit notice in the section editor, and the behavior appears as you mention where the block alert supersedes the modal presentation, but the modal"s still available via toolbar button.

I"ll go ahead and merge so this doesn"t fall too far off main, and we can adjust based on design feedback if needed.

@staykids staykids merged commit 3836da9 into main Feb 10, 2023
@tonisevener tonisevener deleted the block-messages-final-2 branch February 22, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staykids staykids staykids approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tonisevener @staykids @mazevedofs