[NEW] Option to ignore users on channels#10517
Conversation
graywolf336
left a comment
There was a problem hiding this comment.
As stated in this comment, #9785 (comment), the rest api wrapper is needed as well.
|
I'm waiting for the API too (: |
|
just a joke <3 |
graywolf336
left a comment
There was a problem hiding this comment.
Are there plans for the UX/UI piece to be improved?
There was a problem hiding this comment.
typeof ignore === typeof '' seriously? I mean no offense, but are you intentionally trying to make unreadable code?? typeof ignore === 'string' is way more readable than what is currently there..
There was a problem hiding this comment.
I really like this way =/ sorry for make the code more unreadable
| if (!rid || !rid.trim()) { | ||
| throw new Meteor.Error('error-room-id-param-not-provided', 'The required "rid" param is missing.'); | ||
| } | ||
| if (!userId || !userId.trim()) { |
There was a problem hiding this comment.
Add a new line before this. (I'm being nitpicky, but yeah).
There was a problem hiding this comment.
Missing space near the end and the first ending }..
There was a problem hiding this comment.
Should this contain a safety type check of Array.isArray(subscription.ignored)? It would replace the first check since null or undefined isn't an array. Just trying to think in case the data somehow gets corrupted (highly unlikely but yeah)..
There was a problem hiding this comment.
Can you fix this spacing?
As a note, why exactly is this in this pull request?
There was a problem hiding this comment.
because I was working on member list and I saw that problem... maybe I should made an special commit for that
There was a problem hiding this comment.
For readability, can the call to the function be moved to the next line?
There was a problem hiding this comment.
For readability, can the call to the function be moved to the next line?
| Meteor.methods({ | ||
| ignoreUser({rid, userId: ignoredUser, ignore = true}) { | ||
| check(ignoredUser, String); | ||
| check(rid, String); |
There was a problem hiding this comment.
You're type checking everything but the ignore. Is there a good reason behind this or can it be type checked as well?
There was a problem hiding this comment.
just because the name suggest ignore and the param is a boolean so we can define a default... if you dont pass any information I will assume you really want ignore, if you passfalse you want remove the ignore , I dont like the idea of have 2 methods to do the same thing...
There was a problem hiding this comment.
Right, but what if I pass a string to this method? That's what I'm saying, add a type check to ensure the value being passed is a boolean as you assume it always will be.
There was a problem hiding this comment.
hmmm yep you are right :p
|
@graywolf336 yep... we will change this design probably on the RC. Our design team is working on that. |
Closes #9785
as @rodrigok said on #9785
Specification:
Will be a field in room's setting using the component we already have to select users when creating a new channel@gdelavald thanks <3