Skip to content
This repository has been archived by the owner on Jul 9, 2022. It is now read-only.

Add api.changeAdminStatus #659

Merged
merged 1 commit into from
Jul 30, 2018
Merged

Add api.changeAdminStatus #659

merged 1 commit into from
Jul 30, 2018

Conversation

ravkr
Copy link
Contributor

@ravkr ravkr commented Jun 14, 2018

few days ago I've made similar function to #658
But I didn't have time to write docs so I didn't pushed it to Github
It looks a bit cleaner IMO

I've also done some tests in the wild
and you can change adminStatus of every user in thread without being blocked by Facebook

but if you try to do the same thing one by one... Facebook blocks you for an hour :(

obraz

@Schmavery Schmavery mentioned this pull request Jun 14, 2018
Closed
@ravkr
Copy link
Contributor Author

ravkr commented Jun 16, 2018

changelog record and docs added

@ravkr
Copy link
Contributor Author

ravkr commented Jul 14, 2018

@bsansouci, could we merge?

bsansouci
bsansouci suggested changes Jul 15, 2018
View reviewed changes
Copy link
Collaborator

@bsansouci bsansouci left a comment

Choose a reason for hiding this comment

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

Cool looks good to me! We’ll merge as soon as you tweak some of the stuff I commented on

DOCS.md
@@ -22,7 +23,6 @@
* [`api.getThreadPictures`](#getThreadPictures)
* [`api.getUserID`](#getUserID)
* [`api.getUserInfo`](#getUserInfo)
* [`api.threadColors`](#threadColors)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you undo this change? (You moved the function down)

index.js Outdated
'addUserToGroup',
'changeArchivedStatus',
'changeBlockedStatus',
'changeGroupImage',
'changeNickname',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you undo these moves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsansouci that was just sorting alphabetically :/

@ravkr
Copy link
Contributor Author

ravkr commented Jul 15, 2018

These little changes were just sorting alphabetically
Should I really undo them?

@Schmavery
Copy link
Owner

Schmavery commented Jul 24, 2018 •
edited
Loading

@ravkr Alright had a chance to look at this again. If this is the one that you guys want to merge, let's change the name to changeAdminStatus to be more consistent with the other function names and then I'm happy to merge. Idrc about the function order, sorry for the delay here but in the future it's probably simplest to keep it to one PR per feature when possible.

@ravkr ravkr changed the title add editThreadAdmins Add api.changeAdminStatus Jul 28, 2018
@ravkr
Copy link
Contributor Author

ravkr commented Jul 28, 2018

@Schmavery done

@Schmavery Schmavery merged commit 38a619b into Schmavery:master Jul 30, 2018
@Schmavery
Copy link
Owner

thanks!

@Schmavery Schmavery mentioned this pull request Aug 21, 2018
Closed
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@bsansouci bsansouci bsansouci requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ravkr @Schmavery @bsansouci