Page MenuHomePhabricator

Newsletter publisher name not showing up correctly on Manage page.
Closed, ResolvedPublic

Description

Event Timeline

Qgil triaged this task as High priority.Jul 5 2017, 10:35 AM
Qgil added subscribers: nikitavbv, MtDu, Florian and 5 others.

Grrrr we got a last minute bug blocking the deployment of the Newsletter extension!

This feature was implemented in T131492: Better UI for adding / removing Newsletter publishers and it used to work. Either something has changed in OOjs UI or something/someone changed that code.

I am CCing @Phantom42 and other developers involved in the Newsletter, with the hope to get ideas, help... maybe a patch?

We are almost there!!!

I will take a look and try to fix it

After a bit of git bisect-ing, I found out that this bug was introduced after changes made in T166836: Blacklist UsersMultiselectWidget in preferences saves, loads, and renders properly with both no-JS and JS (commit to core). I am currently looking for a way to fix this bug, without breaking any other code, which uses HTMLUsersMultiselectField

Change 363342 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix Newsletter publisher name not showing up correctly on Manage page.

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

The problem was quite simple: we expect default value passed to HTMLUsersMultiselectField to be an array, not a string. But the code written in T166836: Blacklist UsersMultiselectWidget in preferences saves, loads, and renders properly with both no-JS and JS worked with default value as with a string. Looks like we need to check if Blacklist in Echo extension passes users as an array. If it passes users as a string, we need to fix that.

Change 363342 abandoned by Phantom42:
Fix Newsletter publisher name not showing up correctly on Manage page.

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

Change 363358 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/Newsletter@master] Fix Newsletter publisher name not showing up correctly on Manage page.

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

I decided to abandon previous patch because it was breaking other functionality (including Blacklist in Echo extension). I submitted a patch with a better solution: now Newsletter extension passes users as a string of usernames, not an array. After thisn change newsletter manage page is fully functional and no other features are broken.

Change 363358 merged by jenkins-bot:
[mediawiki/extensions/Newsletter@master] Fix Newsletter publisher name not showing up correctly on Manage page.

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

MtDu added a subscriber: Bawolff.

This still happens on other pages, ex. when I edit an old revision.

We can take a look at this later, @Bawolff

Nevermind. After further investigation, it has been determined this issue is fixed on my end.

Confusion was that while addressing https://phabricator.wikimedia.org/T173224, I forgot that we still need to convert what we need (an array of names) into what the HTMLUsersMultiselectField needs to display the names properly (string separated by new lines)

I addressed this using an implode, so everything should be ok.

Reclosing (hopefully forever now, :D)