Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent name collision for teams #83

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
### Changelog

#### 0.8.3 (Next)
#### 0.8.4 (Next)
leechou marked this conversation as resolved.
Show resolved Hide resolved

* Your contribution here.

#### 0.8.3 (2018/11/17)

* [#83](https://github.com/slack-ruby/slack-ruby-bot-server/pull/83): Prevents name collision for Teams, allow configuration of ActiveRecord table name - [@leechou](https://github.com/leechou).

#### 0.8.2 (2018/10/11)

* [#80](https://github.com/slack-ruby/slack-ruby-bot-server/pull/80): Fix: closed stream when closing connection in ping worker - [@dblock](https://github.com/dblock).
Expand Down
35 changes: 27 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ In the examples below, substitute your Github username for `contributor` in URLs

Fork the [project on Github](https://github.com/slack-ruby/slack-ruby-bot-server) and check out your copy.

```
```bash
git clone https://github.com/contributor/slack-ruby-bot-server.git
cd slack-ruby-bot-server
git remote add upstream https://github.com/slack-ruby/slack-ruby-bot-server.git
Expand All @@ -20,7 +20,7 @@ git remote add upstream https://github.com/slack-ruby/slack-ruby-bot-server.git

Make sure your fork is up-to-date and create a topic branch for your feature or bug fix.

```
```bash
git checkout master
git pull upstream master
git checkout -b my-feature-branch
Expand All @@ -30,11 +30,30 @@ git checkout -b my-feature-branch

Ensure that you can build the project and run tests.

```
```bash
export DATABASE_ADAPTER='activerecord'
leechou marked this conversation as resolved.
Show resolved Hide resolved
bundle install
bundle exec rake
```

## Run Tests

Run tests before making changes and then run tests after major commits.
Make sure to test both ActiveRecord and Mongoid.

```bash
export DATABASE_ADAPTER='activerecord'
bundle install
bundle exec rspec
```

Test the sample apps

```bash
cd sample_apps/sample_app_active_record
ln -s ../../lib .
leechou marked this conversation as resolved.
Show resolved Hide resolved
```

## Write Tests

Try to write a test that reproduces the problem you're trying to fix or describes a feature that you want to build.
Expand Down Expand Up @@ -64,21 +83,21 @@ Make it look like every other line, including your name and link to your Github

Make sure git knows your name and email address:

```
```bash
git config --global user.name "Your Name"
git config --global user.email "[email protected]"
```

Writing good commit logs is important. A commit log should describe what changed and why.

```
```bash
git add ...
git commit
```

## Push

```
```bash
git push origin my-feature-branch
```

Expand All @@ -91,7 +110,7 @@ Click the 'Pull Request' button and fill out the form. Pull requests are usually

If you've been working on a change for a while, rebase with upstream/master.

```
```bash
git fetch upstream
git rebase upstream/master
git push origin my-feature-branch -f
Expand All @@ -107,7 +126,7 @@ Update the [CHANGELOG](CHANGELOG.md) with the pull request number. A typical ent

Amend your previous commit and force push the changes.

```
```bash
git commit --amend
git push origin my-feature-branch -f
```
Expand Down
2 changes: 1 addition & 1 deletion DEBUGGING.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ heroku run script/console --app=...

Running `script/console` attached to terminal... up, run.7593

2.2.1 > Team.count
2.2.1 > SlackRubyBotServer::Team.count
=> 3
```
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class MyApp < SlackRubyBotServer::App
private

def deactivate_sleepy_teams!
Team.active.each do |team|
SlackRubyBotServer::Team.active.each do |team|
next unless team.sleepy?
team.deactivate!
end
Expand Down
2 changes: 1 addition & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class MyApp < SlackRubyBotServer::App
private

def deactivate_sleepy_teams!
Team.active.each do |team|
SlackRubyBotServer::Team.active.each do |team|
next unless team.sleepy?
team.deactivate!
end
Expand Down
12 changes: 6 additions & 6 deletions lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class TeamsEndpoint < Grape::API
requires :id, type: String, desc: 'Team ID.'
end
get ':id' do
team = Team.find(params[:id]) || error!('Not Found', 404)
team = SlackRubyBotServer::Team.find(params[:id]) || error!('Not Found', 404)
present team, with: Presenters::TeamPresenter
end

Expand All @@ -22,9 +22,9 @@ class TeamsEndpoint < Grape::API
optional :active, type: Boolean, desc: 'Return active teams only.'
use :pagination
end
sort Team::SORT_ORDERS
sort SlackRubyBotServer::Team::SORT_ORDERS
get do
teams = Team.all
teams = SlackRubyBotServer::Team.all
teams = teams.active if params[:active]
teams = paginate_and_sort_by_cursor(teams, default_sort_order: '-id')
present teams, with: Presenters::TeamsPresenter
Expand All @@ -46,14 +46,14 @@ class TeamsEndpoint < Grape::API
)

token = rc['bot']['bot_access_token']
team = Team.where(token: token).first
team ||= Team.where(team_id: rc['team_id']).first
team = SlackRubyBotServer::Team.where(token: token).first
team ||= SlackRubyBotServer::Team.where(team_id: rc['team_id']).first
if team && !team.active?
team.activate!(token)
elsif team
raise "Team #{team.name} is already registered."
else
team = Team.create!(
team = SlackRubyBotServer::Team.create!(
token: token,
team_id: rc['team_id'],
name: rc['team_name']
Expand Down
8 changes: 4 additions & 4 deletions lib/slack-ruby-bot-server/api/presenters/status_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ module StatusPresenter

def ping
if SlackRubyBotServer::Config.mongoid?
team = Team.asc(:_id).first
team = SlackRubyBotServer::Team.asc(:_id).first
elsif SlackRubyBotServer::Config.activerecord?
team = Team.last
team = SlackRubyBotServer::Team.last
else
raise 'Unsupported database driver.'
end
Expand All @@ -27,11 +27,11 @@ def ping
end

def teams_count
Team.count
SlackRubyBotServer::Team.count
end

def active_teams_count
Team.active.count
SlackRubyBotServer::Team.active.count
end

def base_url(opts)
Expand Down
8 changes: 4 additions & 4 deletions lib/slack-ruby-bot-server/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ def init_database!
end

def mark_teams_active!
Team.where(active: nil).update_all(active: true)
SlackRubyBotServer::Team.where(active: nil).update_all(active: true)
end

def update_team_name_and_id!
Team.active.where(team_id: nil).each do |team|
SlackRubyBotServer::Team.active.where(team_id: nil).each do |team|
begin
auth = team.ping![:auth]
team.update_attributes!(team_id: auth['team_id'], name: auth['team'])
Expand All @@ -50,13 +50,13 @@ def update_team_name_and_id!
def migrate_from_single_team!
return unless ENV.key?('SLACK_API_TOKEN')
logger.info 'Migrating from env SLACK_API_TOKEN ...'
team = Team.find_or_create_from_env!
team = SlackRubyBotServer::Team.find_or_create_from_env!
logger.info "Automatically migrated team: #{team}."
logger.warn "You should unset ENV['SLACK_API_TOKEN']."
end

def purge_inactive_teams!
Team.purge!
SlackRubyBotServer::Team.purge!
end

def configure_global_aliases!
Expand Down
2 changes: 2 additions & 0 deletions lib/slack-ruby-bot-server/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module Config
attr_accessor :server_class
attr_accessor :ping
attr_accessor :database_adapter
attr_accessor :teams_table

def reset!
self.ping = nil
Expand All @@ -16,6 +17,7 @@ def reset!
else
raise 'One of "mongoid" or "activerecord" is required.'
end
self.teams_table = :teams
end

def activerecord?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ def self.check!
end

def self.init!
return if ActiveRecord::Base.connection.tables.include?('teams')
ActiveRecord::Base.connection.create_table :teams do |t|
return if ActiveRecord::Base.connection.tables.include?(SlackRubyBotServer::Config.teams_table.to_s)
ActiveRecord::Base.connection.create_table SlackRubyBotServer::Config.teams_table do |t|
t.string :team_id
t.string :name
t.string :domain
Expand Down
18 changes: 11 additions & 7 deletions lib/slack-ruby-bot-server/models/team/activerecord.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
require_relative 'methods'

class Team < ActiveRecord::Base
include Methods
module SlackRubyBotServer
class Team < ActiveRecord::Base
self.table_name = SlackRubyBotServer::Config.teams_table

def self.purge!
# destroy teams inactive for two weeks
Team.where(active: false).where('updated_at <= ?', 2.weeks.ago).each do |team|
puts "Destroying #{team}, inactive since #{team.updated_at}, over two weeks ago."
team.destroy
include Methods

def self.purge!
# destroy teams inactive for two weeks
SlackRubyBotServer::Team.where(active: false).where('updated_at <= ?', 2.weeks.ago).each do |team|
puts "Destroying #{team}, inactive since #{team.updated_at}, over two weeks ago."
team.destroy
end
end
end
end
4 changes: 2 additions & 2 deletions lib/slack-ruby-bot-server/models/team/methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ def ping!
def self.find_or_create_from_env!
token = ENV['SLACK_API_TOKEN']
return unless token
team = Team.where(token: token).first
team ||= Team.new(token: token)
team = SlackRubyBotServer::Team.where(token: token).first
team ||= SlackRubyBotServer::Team.new(token: token)
info = Slack::Web::Client.new(token: token).team_info
team.team_id = info['team']['id']
team.name = info['team']['name']
Expand Down
30 changes: 16 additions & 14 deletions lib/slack-ruby-bot-server/models/team/mongoid.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
require_relative 'methods'

class Team
include Mongoid::Document
include Mongoid::Timestamps
module SlackRubyBotServer
class Team
include Mongoid::Document
include Mongoid::Timestamps

field :team_id, type: String
field :name, type: String
field :domain, type: String
field :token, type: String
field :active, type: Boolean, default: true
field :team_id, type: String
field :name, type: String
field :domain, type: String
field :token, type: String
field :active, type: Boolean, default: true

include Methods
include Methods

def self.purge!
# destroy teams inactive for two weeks
Team.where(active: false, :updated_at.lte => 2.weeks.ago).each do |team|
Mongoid.logger.info "Destroying #{team}, inactive since #{team.updated_at}, over two weeks ago."
team.destroy
def self.purge!
# destroy teams inactive for two weeks
SlackRubyBotServer::Team.where(active: false, :updated_at.lte => 2.weeks.ago).each do |team|
Mongoid.logger.info "Destroying #{team}, inactive since #{team.updated_at}, over two weeks ago."
team.destroy
end
end
end
end
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot-server/rspec/fabricators/team.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Fabricator(:team) do
Fabricator(:team, from: 'SlackRubyBotServer::Team') do
token { Fabricate.sequence(:team_token) { |i| "abc-#{i}" } }
team_id { Fabricate.sequence(:team_id) { |i| "T#{i}" } }
name { Faker::Lorem.word }
Expand Down
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot-server/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def stop!(team)
end

def start_from_database!
Team.active.each do |team|
SlackRubyBotServer::Team.active.each do |team|
run_callbacks :booting, team
start!(team)
run_callbacks :booted, team
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class CreateTeamsTable < ActiveRecord::Migration[5.0]
def change
create_table :teams, force: true do |t|
create_table SlackRubyBotServer::Config.teams_table, force: true do |t|
t.string :team_id
t.string :name
t.boolean :active, default: true
Expand Down
2 changes: 1 addition & 1 deletion sample_apps/sample_app_activerecord/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# These are extensions that must be enabled in order to support this database
enable_extension 'plpgsql'

create_table 'teams', force: :cascade do |t|
create_table SlackRubyBotServer::Config.teams_table, force: :cascade do |t|
t.string 'team_id'
t.string 'name'
t.boolean 'active', default: true
Expand Down
4 changes: 3 additions & 1 deletion script/console
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#!/usr/bin/env ruby

require_relative '../config/environment'
require 'bundler'

Bundler.require

# Usage: script/console
# Starts an IRB console with slack-ruby-bot-server loaded.
Expand Down
2 changes: 1 addition & 1 deletion spec/api/endpoints/status_endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
include SlackRubyBotServer::Api::Test::EndpointTest

before do
allow_any_instance_of(Team).to receive(:ping!).and_return(ok: 1)
allow_any_instance_of(SlackRubyBotServer::Team).to receive(:ping!).and_return(ok: 1)
end

context 'status' do
Expand Down
Loading