Add Activities creation and retrieval #5

semmons99 · 2012-05-18T01:11:16Z

Adding activities. They are similar to mendibot's topics. However they can be handled after the fact and are not tied to specific rows in the transcript table; instead they are stored in their own activities table.

semmons99 · 2012-05-18T01:13:06Z

@mendicant/owners should we just let anyone who visits the website or irc channel setup their own activity logs?

jordanbyron · 2012-05-18T12:36:40Z

app/anita-web.rb

+  get "/activities/:description", provides: "html" do |description|
+    messages = messages_for_activity(description)
+    render_transcript(messages)
+  end


We should be able to merge these two methods using optional parameters:

get "/activities/:description.?:format?" do messages = messages_for_activity(params[:description]) format = format_for(params[:format] || 'html') render_transcript(messages, format) end

Also, do you think the route should be namespaced to a certain channel? For example:

/:channel/activities/:description.?:format?

I know right now we are only using anita in one channel, but since we are already using that format for general logs, I think it makes sense to stay consistent and do the same thing for activities

I'll have to try merging those two get routes again; I tried before when we were using sinatra-respond_to and it wasn't working.

I omitted the channel in the route so that a user would only need to know the name of the activity and not the channel to find it. I could go either way with requiring it.

@jordanbyron: It's not quite so easy to merge these two get's as one might suspect. The problem is the eager matching of the underlying regex. I've come up with three solutions.

Leave it as two methods.

Use a custom regex. This is what I've come up with to match the basic transactions rendering in one method %r|/(.*)/([0-9+\-:T]*)\.\.([0-9+\-:T]*)(\..*){0,1}|, something more perverse will have to be used to match descriptions.

Continue to use "/:channel/:from..:to" and use some special logic to rip the extension out of :to if it's present.

I'm not really sure which I like better. Option 1 is the clearest to read, but breaks DRY. Option 2 gets us to one method, but introduces a nasty regex. Option 3 looks nice, but has to have some nasty custom logic due to eager matching. Thoughts; is there an option 4 I've missed?

We could also come up with a matcher that doesn't have this problem. Perhaps:

get "/:channel/:from..:to/transcript.?:ext?"

It doesn't really benefit the users to do this though, which makes me think we should still focus on one of the options above.

Hum, let me pull this down and play around with it during any downtime in my office hours today.

jordanbyron · 2012-05-18T12:42:18Z

From @semmons99

should we just let anyone who visits the website or irc channel setup their own activity logs?

In a perfect world it would be nice to have some very simple permissions which can be managed both through AnitaBot and AnitaWeb. Since that is no small task I don't foresee any issues rolling this with liberal permissions at first. If we detect abuse we can easily lock it down and then roll it back out with more safe guards in place.

semmons99 · 2012-05-18T12:53:09Z

@jordanbyron: That sounds like good plan to me. I'll try and get activities completed this weekend.

jordanbyron · 2012-05-23T14:01:59Z

@semmons99 how about this?

class AnitaWeb < Sinatra::Base
  # Snip ...
  get "/:channel/:from..:to.:format" do
    render_transcript(params)
  end

  get "/:channel/:from..:to", provides: "html" do
    render_transcript(params)
  end

  get "/activities/:description.:format" do
    render_activities(params)
  end

  get "/activities/:description", provides: "html" do
    render_activities(params)
  end

  private

  def render_transcript(options)
    messages = messages_for(options[:channel], options[:from], options[:to])

    render_messages(messages, options)
  end

  def render_activities(options)
    messages = messages_for_activity(options[:description])

    render_messages(messages, options)
  end

  def render_messages(messages, options)
    options[:format] ||= 'html'
    format             = format_for(params[:format])

    case format
    when :html
      haml(:transcript, locals: {messages: messages})
    when :json
      messages.to_json
    when :markdown
      erb(:transcript, locals: {messages: messages})
    else
      raise Sinatra::NotFound
    end
  end
  # Snip ...
end

Full Source

I think this is as good as it gets. It really sucks Sinatra is having trouble parsing the params. Although I'm not terribly surprised. The parameter data we are throwing at it is really yucky /jordanbyron/2012-05-23%2010:00%20UTC..2012-05-24%2014:00%20UTC.json

This at least removes the duplicate code and gives us a little more flexibility in what we want to do with the URL by just passing the parameters through to the render_XXX methods. We could have easily used ordinal methods, but this just looks so much cleaner to me.

I couldn't figure out how to create an activity (besides cracking open the DB and doing it manually) so that part of the code wasn't hand tested. Let me know what you think. 😄

semmons99 · 2012-05-23T16:21:32Z

Thanks @jordanbyron! ✨ ❤️ ✨

Used your refactored code here b882a01

semmons99 · 2012-05-24T14:00:26Z

@jordanbyron thoughts on the most recent commits? Should we push this live?

jordanbyron · 2012-05-24T15:16:46Z

app/anita-web.rb


 require_relative "../lib/anita"

 class AnitaWeb < Sinatra::Base
+  InvalidDateString = BasicObject.new


I think this can be removed, since you already have Anita::Activities:: InvalidDateString defined.

Yeah, that was leftover cruft.

jordanbyron · 2012-05-24T15:35:56Z

@semmons99 I made a few inline comments. There are a few other things that we need to start thinking about:

Database migrations: Right now we don't have a nice way to deploy a new version of Anita and have the database migrated to the most recent schema. I'm not sure the best way to accomplish that, but it's a problem we'll need to solve.
Authentication: I know I said before "do it and if we see abuse we'll fix it", but seeing the code and how wide open Anita will be, it makes me rethink that assertion. We might want to table this branch for a bit and add authentication / authorization to Anita before we "do it live". Thoughts?

It's starting to come along nicely. Nice work 😄

semmons99 · 2012-05-24T15:43:13Z

@jordanbyron:

Database migrations: Right now we don't have a nice way to deploy a new version of Anita and have the database migrated to the most recent schema. I'm not sure the best way to accomplish that, but it's a problem we'll need to solve.

Maybe it's time we just grab an ORM and do this right. I've been hankering to try DataMapper, have you had any experience with it?

Authentication: I know I said before "do it and if we see abuse we'll fix it", but seeing the code and how wide open Anita will be, it makes me rethink that assertion. We might want to table this branch for a bit and add authentication / authorization to Anita before we "do it live". Thoughts?

I think you're right about this too. Do we need Clubhouse brought back, or should we just do it here?

jordanbyron · 2012-05-24T15:47:07Z

@semmons99

Maybe it's time we just grab an ORM and do this right. I've been hankering to try DataMapper, have you had any experience with it?

I haven't but I always like to play with new toys on OSS projects. So if that's something you want to explore, then I fully support you 👍

I think you're right about this too. Do we need Clubhouse brought back, or should we just do it here?

Oh god I think I just broke into a cold sweat 😰 No I don't want to worry about launching and maintaining clubhouse right now. Github authentication (through OmniAuth) and very, very simple authorization structure where accounts are granted moderator permission. We don't even need views to start.

semmons99 · 2012-05-24T15:51:07Z

Oh god I think I just broke into a cold sweat 😰 No I don't want to worry about launching and maintaining clubhouse right now. Github authentication (through OmniAuth) and very, very simple authorization structure where accounts are granted moderator permission. We don't even need views to start.

Thoughts on how we'd go about granting IRC users permission? Should we tie their handle to their Github ID? Should we even trust IRC since anyone can spoof an ID if it's not protected?

jordanbyron · 2012-05-24T15:56:01Z

Thoughts on how we'd go about granting IRC users permission? Should we tie their handle to their Github ID? Should we even trust IRC since anyone can spoof an ID if it's not protected?

Good question. IRC is still something I use that I don't fully understand. There has to be a way where Anita::Bot can verify that a given handle is registered with Freenode before allowing it to be added. But that all seems a bit heavy handed, especially since any handle we are granting the commit bit to should be owned by someone we trust. And I'd hope those people have their nick registered.

But let's make that something we document rather than come up with a complicated systematic enforcement for.

semmons99 · 2012-05-25T14:17:02Z

Instead of trying to bring this pull request forward. I'm going to implement the authentication, then start this over from scratch. This was a good spike to start with though.

jordanbyron reviewed May 18, 2012
View reviewed changes

semmons99 and others added 7 commits May 23, 2012 12:01

update setup:database to initialize activities table

e66d68b

ensure activities description is unique

34b2fca

move messages stuff where it belongs

cea046f

allow users to view activity transcripts

2e24f31

update README.md

268bc0a

add pry to the Gemfile

8e52eea

refactor routes per @jordanbyron's suggestion

b882a01

semmons99 added 2 commits May 24, 2012 09:35

add ability to create new activities on web

634ac40

add activity creation in IRC

a41186c

jordanbyron reviewed May 24, 2012
View reviewed changes

pullin' weeds

8d443ab

semmons99 closed this May 25, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Activities creation and retrieval #5

Add Activities creation and retrieval #5

semmons99 commented May 18, 2012

semmons99 commented May 18, 2012

jordanbyron May 18, 2012

semmons99 May 18, 2012

semmons99 May 23, 2012

semmons99 May 23, 2012

jordanbyron May 23, 2012

jordanbyron commented May 18, 2012

semmons99 commented May 18, 2012

jordanbyron commented May 23, 2012

semmons99 commented May 23, 2012

semmons99 commented May 24, 2012

jordanbyron May 24, 2012

semmons99 May 24, 2012

jordanbyron commented May 24, 2012

semmons99 commented May 24, 2012

jordanbyron commented May 24, 2012

semmons99 commented May 24, 2012

jordanbyron commented May 24, 2012

semmons99 commented May 25, 2012

Add Activities creation and retrieval #5

Add Activities creation and retrieval #5

Conversation

semmons99 commented May 18, 2012

semmons99 commented May 18, 2012

jordanbyron May 18, 2012

Choose a reason for hiding this comment

semmons99 May 18, 2012

Choose a reason for hiding this comment

semmons99 May 23, 2012

Choose a reason for hiding this comment

semmons99 May 23, 2012

Choose a reason for hiding this comment

jordanbyron May 23, 2012

Choose a reason for hiding this comment

jordanbyron commented May 18, 2012

semmons99 commented May 18, 2012

jordanbyron commented May 23, 2012

semmons99 commented May 23, 2012

semmons99 commented May 24, 2012

jordanbyron May 24, 2012

Choose a reason for hiding this comment

semmons99 May 24, 2012

Choose a reason for hiding this comment

jordanbyron commented May 24, 2012

semmons99 commented May 24, 2012

jordanbyron commented May 24, 2012

semmons99 commented May 24, 2012

jordanbyron commented May 24, 2012

semmons99 commented May 25, 2012