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

Fix migrations and some sequel things #457

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions barkeep_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,8 @@ def ensure_required_params(*required_params)
#

get "/autocomplete/authors" do
users = User.filter("`email` LIKE ?", "%#{params[:substring]}%").
or("`name` LIKE ?", "%#{params[:substring]}%").distinct(:email).limit(10)
users = User.filter("email LIKE ?", "%#{params[:substring]}%").
or("name LIKE ?", "%#{params[:substring]}%").distinct(:email).limit(10)
{ :values => users.map { |user| "#{user.name} <#{user.email}>" } }.to_json
end

Expand Down Expand Up @@ -538,7 +538,7 @@ def ensure_required_params(*required_params)
# For testing and styling emails.
# - send_email: set to true to actually send the email for this comment.
get "/dev/latest_comment_email_preview" do
comment = Comment.order(:id.desc).first
comment = Comment.order(Sequel.desc(:id)).first
next "No comments have been created yet." unless comment
Emails.send_comment_email(comment.commit, [comment]) if params[:send_email] == "true"
Emails.comment_email_body(comment.commit, [comment])
Expand All @@ -548,7 +548,7 @@ def ensure_required_params(*required_params)
# - send_email: set to true to actually send the email for this commit.
# - commit: the sha of the commit you want to preview.
get "/dev/latest_commit_email_preview" do
commit = params[:commit] ? Commit.first(:sha => params[:commit]) : Commit.order(:id.desc).first
commit = params[:commit] ? Commit.first(:sha => params[:commit]) : Commit.order(Sequel.desc(:id)).first
next "No commits have been created yet." unless commit
Emails.send_commit_email(commit) if params[:send_email] == "true"
Emails.commit_email_body(commit)
Expand Down
10 changes: 5 additions & 5 deletions lib/admin_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ class BarkeepServer < Sinatra::Base

get "/admin/diagnostics?" do
admin_erb :diagnostics, :locals => {
:most_recent_commit => Commit.order(:id.desc).first,
:most_recent_comment => Comment.order(:id.desc).first,
:most_recent_commit => Commit.order(Sequel.desc(:id)).first,
:most_recent_comment => Comment.order(Sequel.desc(:id)).first,
:repos => MetaRepo.instance.repos.map(&:name),
:failed_email_count => CompletedEmail.filter(:result => "failure").count,
:recently_failed_emails =>
CompletedEmail.filter(:result => "failure").order(:created_at.desc).limit(10).all,
:pending_comments => Comment.filter(:has_been_emailed => false).order(:id.asc).limit(10).all,
CompletedEmail.filter(:result => "failure").order(Sequel.desc(:created_at)).limit(10).all,
:pending_comments => Comment.filter(:has_been_emailed => false).order(Sequel.asc(:id)).limit(10).all,
:pending_comments_count => Comment.filter(:has_been_emailed => false).count,
}
end
Expand Down Expand Up @@ -78,7 +78,7 @@ class BarkeepServer < Sinatra::Base
:name => git_repo.name,
:exists_on_disk => !!grit_repo,
:origin_url => origin,
:newest_commit => git_repo.commits_dataset.order(:date.desc).first
:newest_commit => git_repo.commits_dataset.order(Sequel.desc(:date)).first
}
end

Expand Down
20 changes: 10 additions & 10 deletions lib/stats.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@ def self.num_commits(since)
end

def self.num_unreviewed_commits(since)
Commit.filter("`commits`.`date` > ?", since).
Commit.filter("commits.date > ?", since).
left_join(:comments, :commit_id => :commits__id).filter(:comments__id => nil).count
end

def self.num_reviewed_without_lgtm_commits(since)
Commit.filter("`commits`.`date` > ?", since).filter(:approved_by_user_id => nil).
left_join(:comments, :commit_id => :commits__id).filter("`comments`.`id` IS NOT NULL").count
Commit.filter("commits.date > ?", since).filter(:approved_by_user_id => nil).
left_join(:comments, :commit_id => :commits__id).filter("comments.id IS NOT NULL").count
end

def self.num_lgtm_commits(since)
Commit.filter("`commits`.`date` > ?", since).filter("approved_by_user_id IS NOT NULL").count
Commit.filter("commits.date > ?", since).filter("approved_by_user_id IS NOT NULL").count
end

def self.chatty_commits(since)
dataset = Commit.
join(:comments, :commit_id => :id).
filter("`comments`.`created_at` > ?", since).
filter("comments.created_at > ?", since).
join(:git_repos, :id => :commits__git_repo_id).
group_and_count(:commits__sha, :git_repos__name___repo).order(:count.desc).limit(10)
group_and_count(:commits__sha, :git_repos__name___repo).order(Sequel.desc(:count)).limit(10)
commits_sha_repo_count = dataset.all
commits_and_counts = commits_sha_repo_count.map do |sha_repo_count|
grit_commit = MetaRepo.instance.grit_commit(sha_repo_count[:repo], sha_repo_count[:sha])
Expand All @@ -43,17 +43,17 @@ def self.chatty_commits(since)

def self.top_reviewers(since)
user_ids_and_counts = User.join(:comments, :user_id => :id).
filter("`comments`.`created_at` > ?", since).
group_and_count(:users__id).order(:count.desc).limit(10).all
filter("comments.created_at > ?", since).
group_and_count(:users__id).order(Sequel.desc(:count)).limit(10).all
user_ids_and_counts.map do |id_and_count|
[User[id_and_count[:id]], id_and_count[:count]]
end
end

def self.top_approvers(since)
user_ids_and_counts = User.join(:commits, :approved_by_user_id => :id).
filter("`commits`.`approved_at` > ?", since).
group_and_count(:users__id).order(:count.desc).limit(10).all
filter("commits.approved_at > ?", since).
group_and_count(:users__id).order(Sequel.desc(:count)).limit(10).all
user_ids_and_counts.map do |id_and_count|
[User[id_and_count[:id]], id_and_count[:count]]
end
Expand Down
2 changes: 1 addition & 1 deletion migrations/20110723164631_initial.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
unique [:git_repo_id, :sha], :name => "sha_is_unique_per_repo"
text :message
foreign_key :user_id, :users, :key => :id
datetime :date
DateTime :date
boolean :approved, :default => false
end
create_table(:commit_files) do
Expand Down
2 changes: 1 addition & 1 deletion migrations/20110730115408_add_saved_searches_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
up do
create_table(:saved_searches) do
primary_key :id
datetime :created_at
DateTime :created_at
boolean :email_changes, :default => false
end

Expand Down
4 changes: 2 additions & 2 deletions migrations/20110730130858_create_a_default_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
# Creating a default user so we can assume there's a user logged in, for now.
Sequel.migration do
up do
DB[:users].insert(:name => "demo", :email => "[email protected]")
self[:users].insert(:name => "demo", :email => "[email protected]")
end

down do
DB[:users].filter(:email => "[email protected]").delete
self[:users].filter(:email => "[email protected]").delete
end
end
4 changes: 2 additions & 2 deletions migrations/20110802230834_add_comments_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
text :text
int :line_number
String :file_version
datetime :created_at
datetime :updated_at
DateTime :created_at
DateTime :updated_at
end
end

Expand Down
6 changes: 3 additions & 3 deletions migrations/20110804170453_add_user_order_index_to_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
end

# Keep the current ordering (created_at)
DB[:users].each do |user|
DB[:saved_searches].filter(:user_id => user[:id]).order(:created_at).each_with_index do |search, i|
DB[:saved_searches].filter(:id => search[:id]).update(:user_order => i)
self[:users].each do |user|
self[:saved_searches].filter(:user_id => user[:id]).order(:created_at).each_with_index do |search, i|
self[:saved_searches].filter(:id => search[:id]).update(:user_order => i)
end
end

Expand Down
4 changes: 2 additions & 2 deletions migrations/20110818235437_add_email_tasks_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
up do
create_table(:email_tasks) do
primary_key :id
datetime :created_at
datetime :last_attempted
DateTime :created_at
DateTime :last_attempted

String :to, :size => 256
String :subject, :size => 256
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
change do
add_column :email_tasks, :commit_id, Integer
# Delete any outstanding EmailTasks in your database. They're no good without a commit_id.
DB[:email_tasks].delete
self[:email_tasks].delete
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Sequel.migration do
change do
add_column :comments, :has_been_emailed, TrueClass, :default => 0
add_column :comments, :has_been_emailed, TrueClass, :default => false
add_index :comments, [:has_been_emailed, :created_at]
end
end
4 changes: 2 additions & 2 deletions migrations/20111001155432_add_integration_test_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

Sequel.migration do
up do
DB[:users].insert(:name => "Integration test", :email => "[email protected]",
self[:users].insert(:name => "Integration test", :email => "[email protected]",
:stats_time_range => "month", :saved_search_time_period => 7)
end

down do
DB[:users].filter(:email => "[email protected]").delete
self[:users].filter(:email => "[email protected]").delete
end
end
2 changes: 0 additions & 2 deletions migrations/20120128151243_remove_commit_user_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

Sequel.migration do
up do
# Sequel has no database-independent way of dropping a foreign key constraint.
run "ALTER TABLE `commits` DROP FOREIGN KEY `commits_ibfk_2`"
alter_table(:commits) do
drop_column :user_id

Choose a reason for hiding this comment

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

This should be drop_foreign_key :user_id which will also drop the column. MySQL does not automatically drop foreign keys. Also in the down action it should be add_foreign_key :user_id, :users, :key => :id according to Sequel docs:

For foreign key columns, the column in the associated table that this column references. Unnecessary if this column references the primary key of the associated table, except if you are using MySQL.

Copy link
Author

Choose a reason for hiding this comment

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

I moved away from Barkeep, but you are welcome to pick this up.

Choose a reason for hiding this comment

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

What alternative did you choose in the end? The barkeep repo seems pretty much unmaintained right now.

Copy link
Author

Choose a reason for hiding this comment

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

naforo.com (disclaimer I am the author)

Choose a reason for hiding this comment

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

Looks great and a git-hook based tool should work fine with our git repos which are coupled to our Redmine which handles auth/auth. I've dropped you an invite request from my company email.

OK, enough off-topic talk :-)

end
Expand Down
2 changes: 1 addition & 1 deletion migrations/20120324162814_delete_old_demo_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
up do
# We created this user in the migration "20110730130858_create_a_default_user.rb". We no longer need it
# now that we have real user accounts and proper login support.
DB[:users].filter(:email => "[email protected]").delete
self[:users].filter(:email => "[email protected]").delete
end
end
4 changes: 2 additions & 2 deletions migrations/20120428064732_add_user_api_key_and_api_secret.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
add_column :api_secret, String, :default => "", :null => false
end
# Assign a key and secret to all existing users.
DB[:users].all do |user|
DB[:users][:id => user[:id]] = {
self[:users].all do |user|
self[:users][:id => user[:id]] = {
:api_key => Api.generate_user_key,
:api_secret => Api.generate_user_key
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

Sequel.migration do
up do
DB[:users].insert(:name => "Deleted user", :email => "[email protected]",
self[:users].insert(:name => "Deleted user", :email => "[email protected]",
:deleted_at => Time.parse("2013-01-02"))
end

down do
DB[:users].filter(:email => "[email protected]").delete
self[:users].filter(:email => "[email protected]").delete
end
end
2 changes: 1 addition & 1 deletion models/saved_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def comma_separated_list(list)
def map_authors_names(authors_list)
authors_list.map do |author|
if author =~ /^<.*>$/
users = User.filter("`email`=?", author.gsub(/^<|>$/,"")).limit(10).all
users = User.filter("email=?", author.gsub(/^<|>$/,"")).limit(10).all
next users[0].name if users.length > 0
end
author
Expand Down
2 changes: 1 addition & 1 deletion models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# A logged-in user, or the demo user.
# For demo users, we store their saved searches in their cookie instead of the database.
class User < Sequel::Model
one_to_many :saved_searches, :order => [:user_order.desc]
one_to_many :saved_searches, :order => [Sequel.desc(:user_order)]
one_to_many :comments
one_to_many :authors
add_association_dependencies :authors => :destroy
Expand Down