Skip to content

Commit

Permalink
Further refactor diffing code
Browse files Browse the repository at this point in the history
Only diff in one direction, vary argument order to change results.
Easier to read (I hope!).
  • Loading branch information
duncanjbrown committed Jul 21, 2022
1 parent 5f8277c commit 68885e9
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 34 deletions.
61 changes: 33 additions & 28 deletions lib/dfe/analytics/fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,47 +11,52 @@ def self.allowlist
DfE::Analytics.allowlist
end

def self.database
DfE::Analytics.all_entities_in_application
.reduce({}) do |list, entity|
attrs = DfE::Analytics.model_for_entity(entity).attribute_names
list[entity] = attrs

list
end
end

def self.generate_blocklist
diff_model_attributes_against(allowlist)[:missing]
diff_between(database, allowlist)
end

def self.unlisted_fields
diff_model_attributes_against(allowlist, blocklist)[:missing]
diff_between(database, allowlist, blocklist)
end

def self.surplus_fields
diff_model_attributes_against(allowlist)[:surplus]
diff_between(allowlist, database)
end

def self.conflicting_fields
allowlist.keys.reduce({}) do |conflicts, entity|
intersection = Array.wrap(blocklist[entity]) & allowlist[entity]

conflicts[entity] = intersection if intersection.any?

conflicts
end
diff_between(allowlist, diff_between(allowlist, blocklist))
end

def self.diff_model_attributes_against(*lists)
DfE::Analytics.all_entities_in_application
.reduce({ missing: {}, surplus: {} }) do |diff, entity|
attributes_considered = lists.map do |list|
# for each list of model attrs, look up the attrs for this model
list.fetch(entity, [])
end.reduce(:concat)

model = DfE::Analytics.model_for_entity(entity)

missing_attributes = model.attribute_names - attributes_considered
surplus_attributes = attributes_considered - model.attribute_names

diff[:missing][entity] = missing_attributes if missing_attributes.any?

diff[:surplus][entity] = surplus_attributes if surplus_attributes.any?
# extract and concatenate the fields associated with an entity in 1 or
# more entity->field lists
def self.extract_entity_attributes_from_lists(entity, *lists)
lists.map do |list|
list.fetch(entity, [])
end.reduce(:|)
end

diff
end
# returns keys and values present in leftmost list and not present in any
# of the other lists
#
# diff_between({a: [1, 2]}, {a: [2, 3]}) => {a: [1]}
def self.diff_between(primary_list, *lists_to_compare)
primary_list.reduce({}) do |diff, (entity, attrs_in_primary_list)|
attrs_in_lists_to_compare = extract_entity_attributes_from_lists(entity, *lists_to_compare)
differing_attrs = attrs_in_primary_list - attrs_in_lists_to_compare
diff[entity] = differing_attrs if differing_attrs.any?

diff
end
end
end
end
Expand Down
24 changes: 18 additions & 6 deletions spec/dfe/analytics/fields_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,25 @@
end

describe '.conflicting_fields' do
let(:existing_allowlist) { { candidates: %w[email_address id], institutions: %w[id] } }
let(:existing_blocklist) { { candidates: %w[email_address] } }
context 'when fields conflict' do
let(:existing_allowlist) { { candidates: %w[email_address id first_name], institutions: %w[id] } }
let(:existing_blocklist) { { candidates: %w[email_address first_name] } }

it 'returns fields in the model that appear in both lists' do
conflicts = described_class.conflicting_fields
expect(conflicts.keys).to eq(%i[candidates])
expect(conflicts[:candidates]).to eq(['email_address'])
it 'returns the conflicting fields' do
conflicts = described_class.conflicting_fields
expect(conflicts.keys).to eq(%i[candidates])
expect(conflicts[:candidates]).to eq(%w[email_address first_name])
end
end

context 'when there are no conflicts' do
let(:existing_allowlist) { { candidates: %w[email_address], institutions: %w[id] } }
let(:existing_blocklist) { { candidates: %w[id] } }

it 'returns nothing' do
conflicts = described_class.conflicting_fields
expect(conflicts).to be_empty
end
end
end

Expand Down

0 comments on commit 68885e9

Please sign in to comment.