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

add internal_name to service #202

Merged
merged 1 commit into from
Sep 11, 2014
Merged
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
7 changes: 6 additions & 1 deletion app/models/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class Service < ActiveRecord::Base
serialize :volumes, Array

before_save :resolve_name_conflicts
before_create :set_internal_name
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the significance of this line is definitely worth exercising in your tests. Specifically the fact that this internal_name is not updated on subsequent changes (updates)... Hit me up if you want me to ellaborate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to add another testing case.

after_destroy :shutdown

validates_presence_of :name
Expand All @@ -39,7 +40,7 @@ class Service < ActiveRecord::Base
attr_writer :manager

def unit_name
"#{name}.service"
"#{internal_name}.service"
end

def service_state
Expand Down Expand Up @@ -119,4 +120,8 @@ def resolve_name_conflicts
sanitized_name = sanitize_name(name)
self.name = increment_name(sanitized_name)
end

def set_internal_name
self.internal_name = self.name
end
end
15 changes: 15 additions & 0 deletions db/migrate/20140910190904_add_internal_name_to_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class AddInternalNameToService < ActiveRecord::Migration

def up
add_column :services, :internal_name, :string
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not well-versed in the migration stuff, but I'm thinking that we probably also need to do some sort of data manipulation as part of this. If the user already has services in the their DB we'll want to update all those records and copy the name field to the internal_name field. Not sure if that logic is appropriate to put in the migration file or if it goes someplace else. Maybe @alexwelch can weigh-in.


Service.all.each do |s|
s.update_attribute(:internal_name, s.name)
end
end

def down
remove_column :services, :internal_name
end

end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20140528192410) do
ActiveRecord::Schema.define(version: 20140910190904) do

create_table "app_categories", force: true do |t|
t.string "name"
Expand Down Expand Up @@ -86,6 +86,7 @@
t.text "volumes"
t.string "type"
t.integer "app_id"
t.string "internal_name"
end

add_index "services", ["app_id"], name: "index_services_on_app_id"
Expand Down
1 change: 1 addition & 0 deletions spec/builders/app_builder/from_image_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
service = app.services.first

expect(service.name).to eq options[:image]
expect(service.internal_name).to eq options[:image]
expect(service.from).to eq options[:image]
end
end
Expand Down
20 changes: 18 additions & 2 deletions spec/models/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
it_behaves_like 'a classifiable model'

describe '#unit_name' do
it 'postfixes the name with .service' do
subject.name = 'wordpress'
it 'postfixes the internal_name with .service' do
subject.internal_name = 'wordpress'
expect(subject.unit_name).to eq 'wordpress.service'
end
end
Expand All @@ -79,6 +79,21 @@
end
end

describe 'before_create callback' do
describe '#internal_name' do
it 'is set to the service name' do
service = described_class.create(name: 'foobar')
expect(service.internal_name).to eq(service.name)
end

it 'does not change with subsequent updates to the service' do
service = described_class.create(name: 'foobar')
service.update(name: 'new_name')
expect(service.internal_name).to eq('foobar')
end
end
end

describe 'after initialization' do

describe '#name=' do
Expand All @@ -105,6 +120,7 @@
end

end

end

describe '#submit' do
Expand Down