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

Add #all method to Lita::User, matching specs #103

Closed
wants to merge 1 commit into from

Conversation

chriswoodrich
Copy link

No description provided.

@chriswoodrich chriswoodrich force-pushed the all-users branch 2 times, most recently from 8df4b40 to f369594 Compare May 2, 2015 23:11
@jimmycuadra
Copy link
Collaborator

What's the use case for this?

@chriswoodrich
Copy link
Author

Convenience for when a handler needs access to all the users for messaging
or other functionality.

On Sat, May 2, 2015 at 5:06 PM, Jimmy Cuadra notifications@github.com
wrote:

What's the use case for this?


Reply to this email directly or view it on GitHub
#103 (comment).

# Since 4.2.3
def all
ids = redis.keys.select { |key| key.include? "id" }.map { |id| id.split(":").last }.sort
ids.each.map { |id| find_by_id(id) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be ids.map { |id| find_by_id(id) }

.each.map is incorrect

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @webdestroya

@jimmycuadra
Copy link
Collaborator

I think we discussed this before, but unless there's something I'm not understanding about the use case, authorization groups are probably a better fit sending out mass messages. That way you can scope it to the purpose of the specific message and people it's relevant for.

It's also worth noting that there will eventually be support for a "roster" feature that will tell you who is currently online, filterable by room.

@jimmycuadra
Copy link
Collaborator

Thanks for this, but I think I'm going to close it in favor of a proper roster feature, which should be coming relatively soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants