Skip to content

Commit

Permalink
lint
Browse files Browse the repository at this point in the history
  • Loading branch information
caitmich committed Nov 1, 2024
1 parent f607a07 commit 3c364c4
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 54 deletions.
12 changes: 2 additions & 10 deletions app/models/concerns/file_backed_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# http://asciicasts.com/episodes/219-active-model
# https://github.com/rails/rails/blob/master/activemodel/lib/active_model/conversion.rb
module FileBackedModel

class FileNotFoundException < Exception; end

def self.included(base)
Expand All @@ -25,8 +24,6 @@ def self.included(base)
end
end



module ClassMethods
# ------------------------------------------------------- Common attributes
# The default file extension to attach to each saved file.
Expand Down Expand Up @@ -56,12 +53,10 @@ def set_pwd(path)
@default_path = path
end


# ---------------------------------------------------- ActiveRecord finders

# Find by :id, which in this case is the file's basename
def find(id)

# Discard any input with weird characters
if (id =~ /\A[\x00\/\\:\*\?\"<>\|]\z/)
raise Exception.new('Not found!')
Expand All @@ -87,10 +82,8 @@ def all()
self.from_file(file)
end.sort
end

end # /ClassMethods


# --------------------------------------------------------- ActiveModel::Lint

# ActiveModel expects you to define an id() method to uniquely identify each
Expand All @@ -107,7 +100,6 @@ def persisted?()
@persisted ||= File.exist?(full_path)
end


# ---------------------------------------------------------------- Enumerable

# When comparing two NoteTemplate instances, sort them alphabetically on their
Expand All @@ -119,7 +111,7 @@ def <=>(other)
# -------------------------------------------------- Constructors & Lifecycle

# Constructor a la ActiveRecord. Attributes: :name, :file
def initialize(attributes={})
def initialize(attributes = {})
attributes.each do |name, value|
send("#{name}=", value)
end
Expand Down Expand Up @@ -158,6 +150,6 @@ def name
end

def name=(new_name)
self.filename= new_name.gsub(/ /,'_').underscore
self.filename = new_name.gsub(/ /, '_').underscore
end
end
4 changes: 2 additions & 2 deletions app/models/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def nested_activities
}

scope :user_nodes, -> {
where("type_id IN (?)", Types::USER_TYPES)
where('type_id IN (?)', Types::USER_TYPES)
}

# -- Class Methods --------------------------------------------------------
Expand All @@ -84,7 +84,7 @@ def ancestor_of?(node)

# Return all the Attachment objects associated with this Node.
def attachments
Attachment.find(:all, :conditions => {:node_id => self.id})
Attachment.find(:all, conditions: { node_id: self.id })
end

def user_node?
Expand Down
4 changes: 2 additions & 2 deletions app/services/naming_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def self.name_file(original_filename:, pathname:)
def self.name_project(name)
return name unless Project.exist?(name: name)

projects = Project.where("name LIKE ?", "#{name}_copy-%")
projects = Project.where('name LIKE ?', "#{name}_copy-%")
project_names = projects.map(&:name)

new_name(
Expand All @@ -40,7 +40,7 @@ def self.name_project(name)
private

def self.new_name(name:, sequence:, suffix: nil)
"%s_copy-%02i%s" % [name, sequence, suffix]
'%s_copy-%02i%s' % [name, sequence, suffix]
end

def self.next_sequence(matching_names: [], suffix: nil)
Expand Down
4 changes: 2 additions & 2 deletions lib/tasks/thor/setup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ def configure
desc 'kit', 'Import files and projects from a specified Kit configuration file'
method_option :file, required: true, type: :string, desc: 'full path to the Kit file to use.'
def kit
puts "** Importing kit..."
puts '** Importing kit...'
KitImportJob.perform_now(options[:file], logger: default_logger)
puts "[ DONE ]"
puts '[ DONE ]'
end

desc 'welcome', 'adds initial content to the repo for demonstration purposes'
Expand Down
38 changes: 17 additions & 21 deletions spec/lib/file_backed_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,21 @@ class BasicFileBackedModel
end

subject { ::BasicFileBackedModel.new }
let(:pwd){ BasicFileBackedModel.pwd }
let(:rspec_file){ pwd.join('rspec.txt')}
it_behaves_like "ActiveModel"
let(:pwd) { BasicFileBackedModel.pwd }
let(:rspec_file) { pwd.join('rspec.txt') }
it_behaves_like 'ActiveModel'

after(:all) do
FileUtils.rm_rf('tmp/templates/')
::Configuration::where(name: 'admin:paths:rspec').limit(1).destroy_all
end


# ------------------------------------------------------- Class configuration
describe '.extension' do
it "defaults to a .txt extension" do
it 'defaults to a .txt extension' do
expect(BasicFileBackedModel.extension).to eq('.txt')
end
it "allows including classes to define their own" do
it 'allows including classes to define their own' do
class ExtensionFBM
include FileBackedModel
set_extension :xml
Expand All @@ -36,12 +35,11 @@ class ExtensionFBM
end
end


# ----------------------------------------------------------- File operations
describe '.from_file' do
it "loads an instance based on the file on disk" do
it 'loads an instance based on the file on disk' do
FileUtils.mkdir_p(pwd) unless File.exist?(pwd)
File.open(rspec_file, 'w'){|f| f << 'bar'}
File.open(rspec_file, 'w') { |f| f << 'bar' }

expect(subject.class).to respond_to(:from_file)
# expect(subject.class.from_file).to raise_error
Expand All @@ -61,9 +59,9 @@ class ExtensionFBM
expect(instance.destroy).to be true
end

it "deletes the file from disk if this is a file-backed model" do
it 'deletes the file from disk if this is a file-backed model' do
FileUtils.mkdir_p(pwd) unless File.exist?(pwd)
File.open(rspec_file, 'w'){|f| f << 'foobar' }
File.open(rspec_file, 'w') { |f| f << 'foobar' }

expect(File.exist?(rspec_file)).to be true
instance = subject.class.from_file(rspec_file)
Expand All @@ -73,7 +71,7 @@ class ExtensionFBM
end

describe '#save' do
pending "generates a filename using the #name if provided" do
pending 'generates a filename using the #name if provided' do
subject.name = 'Singing in the rain'
should be true

Expand All @@ -89,7 +87,7 @@ class ExtensionFBM
# it "auto-generates a filename if no #name was provided" do
# end

it "overwrites the contents of the file on disk" do
it 'overwrites the contents of the file on disk' do
subject.name = 'rspec name'
subject.content = 'barfoo'
expect(subject.save).to be true
Expand All @@ -100,14 +98,13 @@ class ExtensionFBM
end
end


# ------------------------------------------------------------------- Finders
describe '.all' do
it "returns a new instance for each file in the .pwd" do
it 'returns a new instance for each file in the .pwd' do
FileUtils.mkdir_p(pwd) unless File.exist?(pwd)

(1..3).each do |i|
File.open(pwd.join("#{i}.txt"), 'w'){|f| f << 'foo' }
File.open(pwd.join("#{i}.txt"), 'w') { |f| f << 'foo' }
end

expect(BasicFileBackedModel.all.count).to eq(3)
Expand All @@ -122,9 +119,9 @@ class ExtensionFBM
end.to raise_exception(FileBackedModel::FileNotFoundException)
end

it "loads an instance based on the file name" do
it 'loads an instance based on the file name' do
FileUtils.mkdir_p(pwd) unless File.exist?(pwd)
File.open(rspec_file, 'w'){|f| f << 'bar'}
File.open(rspec_file, 'w') { |f| f << 'bar' }

expect(subject.class).to respond_to(:find)

Expand All @@ -138,12 +135,11 @@ class ExtensionFBM
end
end


# ---------------------------------------------------------- Instance methods
describe '#name' do
it "responds to #name based on the filename" do
it 'responds to #name based on the filename' do
FileUtils.mkdir_p(pwd) unless File.exist?(pwd)
File.open(rspec_file, 'w'){|f| f << 'bar' }
File.open(rspec_file, 'w') { |f| f << 'bar' }

bfbm = BasicFileBackedModel.from_file(rspec_file)
expect(bfbm).to respond_to(:name)
Expand Down
10 changes: 5 additions & 5 deletions spec/models/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@
end

it 'ignores same value for hash properties (same key type)' do
node.set_property(:test_property, {port: 80, protocol: 'tcp'})
node.set_property(:test_property, {port: 80, protocol: 'tcp'})
node.set_property(:test_property, { port: 80, protocol: 'tcp' })
node.set_property(:test_property, { port: 80, protocol: 'tcp' })

# Because we're getting a WithIndifferentAccess hash, can't compare
# directly.
Expand All @@ -163,8 +163,8 @@
# strings for DB serialisation. If we start sending Symbols, they
# become Strings and by the time we're setting the second property
# we'd be comparing strings with strings.
node.set_property(:test_property, {'port' => 80, 'protocol' => 'tcp'})
node.set_property(:test_property, {port: 80, protocol: 'tcp'})
node.set_property(:test_property, { 'port' => 80, 'protocol' => 'tcp' })
node.set_property(:test_property, { port: 80, protocol: 'tcp' })

# Because we're getting a WithIndifferentAccess hash, can't compare
# directly.
Expand Down Expand Up @@ -198,7 +198,7 @@

it 'raises if you try to set :services or services_extras' do
expect do
node.set_property(:services, [{port: '22', protocol: 'tcp'}])
node.set_property(:services, [{ port: '22', protocol: 'tcp' }])
end.to raise_error(ArgumentError, /set_service/)

expect do
Expand Down
23 changes: 11 additions & 12 deletions spec/models/note_template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
FileUtils.rm_rf('tmp/templates/')
end

it "defines a class .pwd() method" do
it 'defines a class .pwd() method' do
expect(NoteTemplate::methods).to include(:pwd)
expect(NoteTemplate.pwd).to respond_to('join')
end

it "provides a find(id) method" do
it 'provides a find(id) method' do
expect(NoteTemplate::methods).to include(:find)

# Handle non-existing
Expand All @@ -27,14 +27,14 @@
# Handle existing templates
FileUtils.mkdir_p(NoteTemplate.pwd) unless File.exist?(NoteTemplate.pwd)
test_file = NoteTemplate.pwd.join('foo.txt')
File.open( test_file, 'w' ){ |f| f << 'bar' }
File.open(test_file, 'w') { |f| f << 'bar' }
expect { NoteTemplate.find('foo') }.to_not raise_error()
nt = NoteTemplate.find('foo')
expect(nt.content).to eq('bar')
File.delete(test_file)
end

it "validates presence of filename" do
it 'validates presence of filename' do
nt = NoteTemplate.new(content: 'foobar')
expect(nt).not_to be_valid()

Expand All @@ -49,23 +49,23 @@
end

it "doesn't accept bad characters in the name" do
nt = NoteTemplate.new(name: '../../../../../etc' )
nt = NoteTemplate.new(name: '../../../../../etc')
expect(nt).to_not be_valid()

nt.name = "foo"
nt.name = 'foo'
expect(nt).to be_valid()

nt.name = 'bar.txt'
expect(nt).to_not be_valid()
end

it "needs to be valid before it is saved on disk" do
it 'needs to be valid before it is saved on disk' do
nt = NoteTemplate.new
expect(nt).to_not be_valid()
expect(nt.save).to be false
end

pending "prevents a template from being overwritten"
pending 'prevents a template from being overwritten'

it "creates the templates dir if it doesn't exist when saving" do
FileUtils.rm_rf(NoteTemplate.pwd) if File.exist?(NoteTemplate.pwd)
Expand All @@ -80,8 +80,7 @@
File.delete(new_note_template)
end


it "saves the template contents when saving the instance" do
it 'saves the template contents when saving the instance' do
nt = NoteTemplate.new(content: 'FooBar', filename: 'tpl_test')
expect(nt.save).to be true
filename = NoteTemplate.pwd.join('tpl_test.txt')
Expand All @@ -91,7 +90,7 @@
File.delete(filename)
end

it "deletes file from disk on destroy" do
it 'deletes file from disk on destroy' do
nt = NoteTemplate.new(content: 'FooBar', filename: 'tpl_test')
nt.save

Expand All @@ -109,7 +108,7 @@

FileUtils.mkdir_p(NoteTemplate.pwd) unless File.exist?(NoteTemplate.pwd)
filename = NoteTemplate.pwd.join('foobar.txt')
File.open(filename,'w'){ |f| f<<'barfoo' }
File.open(filename, 'w') { |f| f << 'barfoo' }

nt = NoteTemplate.from_file(filename)
expect(nt.content).to eq('barfoo')
Expand Down

0 comments on commit 3c364c4

Please sign in to comment.