Skip to content
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ bower.json
*.swp
*.swo
.vimrc

config/database.yml
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ gem 'sass-rails', '~> 5.0'
gem 'sextant', group: [:development]
gem 'slim-rails'
gem 'spring', group: [:development]
gem 'spring-commands-rspec', group: [:test]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

чтобы из Vim можно было запускать тесты

gem 'spring-watcher-listen', '~> 2.0.0', group: [:development]
gem 'turbolinks', '~> 5'
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ GEM
railties (>= 3.1)
slim (~> 3.0)
spring (1.7.2)
spring-commands-rspec (1.0.4)
spring (>= 0.9.1)
spring-watcher-listen (2.0.0)
listen (>= 2.7, < 4.0)
spring (~> 1.2)
Expand Down Expand Up @@ -286,6 +288,7 @@ DEPENDENCIES
shoulda-matchers (~> 3.1)
slim-rails
spring
spring-commands-rspec
spring-watcher-listen (~> 2.0.0)
turbolinks (~> 5)
tzinfo-data
Expand Down
8 changes: 8 additions & 0 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,11 @@ label.required:after {
content: " *";
color: red;
}

.form-inline {
.form-group {
input {
width: 100%;
}
}
}
37 changes: 37 additions & 0 deletions app/controllers/flows/contacts_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true
class Flows::ContactsController < ApplicationController
def index
@contacts = contacts
@contact = current_user.contacts.build
end

def create
contact = Contact.find_or_initialize_by(email: contact_params[:email])
contact.name = contact_params[:name]

if contact.save
membership = current_user.memberships.build(contact_id: contact.id)
return redirect_to flows_contacts_path, alert: 'Contact has already been added' unless membership.valid?
membership.save
redirect_to flows_contacts_path, notice: 'Contact has been added'
else
@contacts = contacts
@contact = contact
render :index
Copy link
Contributor Author

Choose a reason for hiding this comment

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

не очень нравится, как сделал сохранение. Тут две операции: добавляем контакт, создаем связь. Это две разные модели, у каждой есть своя валидация.
Хотел бы показатьmembership.errors на форме, как например, contacts.errors, чтобы все ошибки показывались в одном месте и одном формате, но не знаю как это сделать. Поэтому, membership.errors показываю как alert.
Как правильно нужно сделать ?

Кстати, еще вопрос: в базе есть контактname: tom, email: tom@gmail.com. Я добавляю контакт name: thomas, email: tom@gmail.com. Т.е. я перезаписываю существующее имя ?

Copy link
Contributor

Choose a reason for hiding this comment

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

  • для такого можно использовать сервис классы + транзакция, но я думаю попозже покажу как это можно рефакторить, в целом стоит посмотреть на методы save! которые вызывают exception и как обернуть такие атомарные операции в 1 транзакцию можно например тут http://vaidehijoshi.github.io/blog/2015/08/18/safer-sql-using-activerecord-transactions/
  • да, имя перезапишется на новое

Copy link
Contributor Author

@max-borisov max-borisov Sep 30, 2016

Choose a reason for hiding this comment

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

@mpakus понял, сервис попробую сделать.
Я думал про транзакции, а потом решил, что даже если юзер сохранился, а связь нет, это не страшно, так как при следующем добавлении этого юзера(а он уже будет в БД) сохраниться только связь.
Ну а вообще, конечно, согласен, что это должно быть одной транзакцией.

end
end

def edit; end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Редактирование нужно, пока не совсем понятны такие моменты:

  • если редактировать email. Я добавил контакт user@gmail.com. После этого, кто-то другой тоже хотел добавить этот же email, но так как он уже в БД, будет сохранена только связь. Если отредактирую email, то это коснется и других, кто тоже добавил этот контакт
  • тоже самое и с именем. Если я поменяю имя контакта, то другие пользователь это заметят :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ничего страшного, agile и подразумевается, что запустил некую фичу за какой-то определенный период (спринт), пошел отзыв, что-то не понравилось переделал, это всегда можно сделать например хранить имена в Membership


def destroy; end
Copy link
Contributor Author

@max-borisov max-borisov Sep 26, 2016

Choose a reason for hiding this comment

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

Будем удалять только саму связь из memberships


private

def contacts
@contacts = current_user.contacts.order(id: :asc)
end

def contact_params
params.require(:contact).permit(:name, :email)
end
end
8 changes: 8 additions & 0 deletions app/models/membership.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true
class Membership < ApplicationRecord
belongs_to :subscriber, class_name: 'User'
belongs_to :subscribed, class_name: 'User'

validates :name, presence: true, length: { maximum: 255 }
validates :subscribed_id, uniqueness: { scope: :subscriber_id }
end
8 changes: 6 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
# frozen_string_literal: true
class User < ApplicationRecord
devise :database_authenticatable, :registerable, :recoverable, :rememberable,
:trackable, :validatable
devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable

has_many :newsletters
has_many :ownerships, class_name: 'Membership', foreign_key: :subscriber_id
has_many :memberships, foreign_key: :subscribed_id

has_many :subscribers, class_name: 'User', through: :memberships, foreign_key: :subscriber_id, dependent: :destroy
has_many :subscribeds, class_name: 'User', through: :ownerships, foreign_key: :subscribed_id, dependent: :destroy

validates :name, presence: true, length: { maximum: 255 }
end
6 changes: 6 additions & 0 deletions app/views/flows/contacts/_contact.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
tr
td= contact.id
td= link_to contact.name, '#'
td= contact.email
td= 0
td= 0
11 changes: 11 additions & 0 deletions app/views/flows/contacts/_form.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.panel.panel-success
.panel-body
form.form-inline
= form_for contact, url: url, html: { class: 'for-inline' } do |f|
= render 'shared/errors', model: contact
.form-group.col-lg-5
= f.text_field :name, placeholder: 'Name', class: 'form-control'
.form-group.col-lg-5
= f.text_field :email, placeholder: 'E-mail', class: 'form-control'
.form-group.col-lg-2
= f.submit 'Добавить', class: 'btn btn-success'
15 changes: 15 additions & 0 deletions app/views/flows/contacts/index.html.slim
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
h1 Мои Контакты

= render 'form', contact: @contact, url: flows_contacts_path

- if @contacts.empty?
.alert.alert-warning.text-center Нет добавленных контактов
-else
table.table.table-hover.table-striped
tr
th ID
th Имя
th E-mail
th Кол-во подписок
th Кол-во отписанных
= render @contacts
File renamed without changes.
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
devise_for :users
namespace :flows do
resources :newsletters, except: [:edit]
resources :contacts, only: [:index, :create]
end
resources :flows, only: [:index, :show]
root to: 'flows#index'
Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20160918121902_create_users_contacts.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class CreateUsersContacts < ActiveRecord::Migration[5.0]
def change
create_table :users_contacts, id: false do |t|
Copy link
Contributor

Choose a reason for hiding this comment

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

Тут глобальная проблема эти join_table нужно называть в порядке отсортированных моделей соединяемых, тогда не надо будет явно указывать join_table contacts_users

3.3.2 Creating Join Tables for has_and_belongs_to_many Associations

If you create a has_and_belongs_to_many association, you need to explicitly create the joining table. Unless the name of the join table is explicitly specified by using the :join_table option, Active Record creates the name by using the lexical book of the class names. So a join between author and book models will give the default join table name of "authors_books" because "a" outranks "b" in lexical ordering.

http://guides.rubyonrails.org/association_basics.html#updating-the-schema

Copy link
Contributor Author

Choose a reason for hiding this comment

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

точно, не обратил внимания

t.belongs_to :user, index: true
t.belongs_to :contact, index: true
end
end
end
10 changes: 10 additions & 0 deletions db/migrate/20160924203226_create_memberships.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class CreateMemberships < ActiveRecord::Migration[5.0]
def change
create_table :memberships do |t|
t.belongs_to :user, index: true
t.belongs_to :contact, index: true

t.timestamps
end
end
end
8 changes: 8 additions & 0 deletions db/migrate/20160925080106_drop_users_contacts_table.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class DropUsersContactsTable < ActiveRecord::Migration[5.0]
def change
drop_table :users_contacts, id: false do |t|
t.belongs_to :user, index: true
t.belongs_to :contact, index: true
end
end
end
5 changes: 5 additions & 0 deletions db/migrate/20160925080729_add_unique_index_to_memberships.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddUniqueIndexToMemberships < ActiveRecord::Migration[5.0]
def change
add_index :memberships, [:user_id, :contact_id], unique: true
end
end
5 changes: 5 additions & 0 deletions db/migrate/20161001091106_add_name_to_memberships.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddNameToMemberships < ActiveRecord::Migration[5.0]
def change
add_column :memberships, :name, :string, limit: 255
end
end
6 changes: 6 additions & 0 deletions db/migrate/20161004163616_rename_columns_in_membership.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class RenameColumnsInMembership < ActiveRecord::Migration[5.0]
def change
rename_column :memberships, :user_id, :subscriber_id
rename_column :memberships, :contact_id, :subscribed_id
end
end
13 changes: 12 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,18 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20160917154254) do
ActiveRecord::Schema.define(version: 20161004163616) do

create_table "memberships", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
t.integer "subscriber_id"
t.integer "subscribed_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "name"
Copy link
Contributor Author

@max-borisov max-borisov Oct 1, 2016

Choose a reason for hiding this comment

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

@mpakus Добавил name

t.index ["subscribed_id"], name: "index_memberships_on_subscribed_id", using: :btree
t.index ["subscriber_id", "subscribed_id"], name: "index_memberships_on_subscriber_id_and_subscribed_id", unique: true, using: :btree
t.index ["subscriber_id"], name: "index_memberships_on_subscriber_id", using: :btree
end

create_table "newsletters", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
t.string "name"
Expand Down
6 changes: 6 additions & 0 deletions spec/factories/membership.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true
FactoryGirl.define do
factory :membership do
name { Faker::Name.name }
end
end
49 changes: 49 additions & 0 deletions spec/models/membership_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true
RSpec.describe Membership, type: :model do
subject { FactoryGirl.build(:membership) }

let(:batman){ FactoryGirl.create(:user, name: 'Batman') }
let(:catwoman){ FactoryGirl.create(:user, name: 'Catwoman') }
let(:spiderman){ FactoryGirl.create(:user, name: 'spiderman') }

context 'with relations' do
it { should belong_to(:subscriber) }
it { should belong_to(:subscribed) }
end

context 'with validations' do
before do
subject.subscriber = batman
subject.subscribed = catwoman
subject.save
end

it { should validate_presence_of(:name) }
it { should validate_length_of(:name).is_at_most(255) }

it 'subscribed user should be uniq for subscriber' do
should validate_uniqueness_of(:subscribed_id).scoped_to(:subscriber_id)
end
end

context 'with membership subscriptions' do
before do
subject.subscriber = batman
subject.subscribed = catwoman
subject.save
end

it 'subscribe 1 person' do
expect(Membership.count).to eq 1
end

it 'subscribe 2 people' do
meme = FactoryGirl.build(:membership)
meme.subscriber = batman
meme.subscribed = spiderman
meme.save

expect(Membership.count).to eq 2
end
end
end
30 changes: 27 additions & 3 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
RSpec.describe User, type: :model do
subject { FactoryGirl.build(:user) }

let(:batman){ FactoryGirl.create(:user, name: 'Batman') }
let(:catwoman){ FactoryGirl.create(:user, name: 'Catwoman') }
let(:spiderman){ FactoryGirl.create(:user, name: 'spiderman') }

context 'with validation' do
it 'checks empty name' do
subject.name = nil
Expand All @@ -28,9 +32,29 @@
end
end

context 'association' do
it 'responds to newsletters association' do
expect(subject).to respond_to(:newsletters)
context 'with associations' do
it { expect(subject).to respond_to(:newsletters) }
it { respond_to(:memberships) }
it { respond_to(:ownerships) }
it { respond_to(:subscribers) }
it { respond_to(:subscribeds) }
end

context 'with memberships' do
before do
batman.ownerships.create!(subscribed: catwoman, name: catwoman.name)
batman.ownerships.create!(subscribed: spiderman, name: spiderman.name)
end

it 'fetch right subscribeds' do
expect(batman.subscribeds.count).to eq 2
expect(batman.subscribeds).to include catwoman
expect(batman.subscribeds).to include spiderman
end

it 'fetch right subscriber' do
expect(catwoman.subscribers.first).to eq batman
expect(spiderman.subscribers.first).to eq batman
end
end
end