Page MenuHomePhabricator

[Unit testing] For an existing newsletter `FooBarExists`, API edits to remove publishers should work
Closed, ResolvedPublic

Description

Follow instructions as per https://phabricator.wikimedia.org/T183817 to mock an API edit an existing Newsletter FooBarExists. This unit test should:

The original newsletter (Newsletter:FooBarExists) can be created without using APIEdits, and just by new Newsletter() with values. Add 3 or 4 publishers to the newsletter firstly.

Try removing publishers by sending an array of users which exists like:

"publishers": [
    "userName",
    "userName1", 
]

etc. On save, make sure that:

  1. these users are not any more publishers of the newsletter

Details

Related Gerrit Patches:
mediawiki/extensions/Newsletter : masterAdd unit test for removing publishers via API

Event Timeline

Pppery updated the task description. (Show Details)Dec 31 2017, 2:11 PM
Pppery added a subscriber: Pppery.

There's no code right now that removed users who were de-publishered from the subscribers list, so removing that from the things this should est.

nikitavbv claimed this task.Jan 1 2018, 2:19 AM
nikitavbv added a subscriber: nikitavbv.

I will add this test case!

nikitavbv removed nikitavbv as the assignee of this task.EditedJan 1 2018, 5:09 PM

While working on this I noticed that this is blocked by https://gerrit.wikimedia.org/r/#/c/401189/ change. There is no sense to work on this until that change is merged. I also see that @Pppery did a lot of work here, so I think it would be better for me to abandon this task and return to it later or @Pppery may finish this. Good luck!

Pppery added a comment.Jan 1 2018, 5:48 PM

Although I have no objection on working on this task, how exactly is it "blocked by https://gerrit.wikimedia.org/r/#/c/401189/"?

Well, for this task we need to add one more test case to NewsletterAPIEditTest.php which is edited in that change pretty much. So obviously it is better to wait for that change to be merged to avoid merge conflicts and some other minor problems. Additionally, I see that there are some problems with Echo extension not being available during CI build. So it is better to wait for that to be solved before adding tests for removing publishers. Hopefully you understand what I mean and agree with those points.

Change 401418 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/Newsletter@master] Add unit test for removing publishers via API

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

Pppery claimed this task.Jan 2 2018, 1:51 AM

Change 401418 merged by jenkins-bot:
[mediawiki/extensions/Newsletter@master] Add unit test for removing publishers via API

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

01tonythomas closed this task as Resolved.Feb 1 2018, 4:44 AM

All patchsets merged, closing now. Thanks.