Generate inline taxonomy mapping in taxonomy.json #123

chesterbot01 · 2024-03-26T01:45:39Z

Generate inline taxonomy mapping:

Taxonomy viewer works as normal

elsom25 · 2024-03-26T12:11:02Z

@chesterbot01 can we start with new dist files for mappings only? Once those are made (and vetted), we can add this to taxonomy.json

elsom25 · 2024-03-26T12:11:46Z

We should also be preparing a fresh schema cue file to validate these for json. I'd also suggest thinking through what (if any) the appropriate txt representation might be

elsom25 · 2024-03-26T12:13:26Z

app/serializers/dist/json_serializer.rb

@@ -70,5 +72,61 @@ def serialize_nested(connection)
        name: connection.name,
      }
    end
+
+    def build_mapping_blocks(mapping_rules)


Use the instance variable instead of passing this in? The other ones take a param because they're operating against a subset of the instance var vs its entirety

elsom25 · 2024-03-26T12:19:08Z

app/serializers/dist/json_serializer.rb

+      Integration.all.each do |integration|
+        [true, false].each do |from_shopify|


Mostly, if you decompose this it'll be easier to read with each piece being more focused. Also makes it simpler to use map methods, which are nice that each with manual tracking arrays 😁

elsom25 · 2024-03-26T12:23:33Z

app/serializers/dist/json_serializer.rb

+          mappings = []
+          Category.verticals.each do |vertical|
+            mappings << MappingBuilder.build_one_to_one_mappings_for_vertical(
+              mapping_rules: mapping_rule_block,
+              vertical: vertical,
+            )
+          end
+          processed_mappings = mappings.flatten.compact


AFAICT compact isn't needed here

Suggested change

mappings = []

Category.verticals.each do |vertical|

mappings << MappingBuilder.build_one_to_one_mappings_for_vertical(

mapping_rules: mapping_rule_block,

vertical: vertical,

)

end

processed_mappings = mappings.flatten.compact

mappings = Category.verticals.flat_map do |vertical|

MappingBuilder.build_one_to_one_mappings_for_vertical(mapping_rules:, vertical:)

end

elsom25 · 2024-03-26T12:27:16Z

app/serializers/dist/json_serializer.rb

+        mapping[:input][:attributes] = mapping[:input][:attributes].map do |attribute|
+          {
+            name: Property.find(attribute[:name]).gid,
+            value: attribute[:value].nil? ? nil : PropertyValue.find(attribute[:value]).gid,


Suggested change

value: attribute[:value].nil? ? nil : PropertyValue.find(attribute[:value]).gid,

value: attribute[:value] || PropertyValue.find(attribute[:value]).gid,

I have to revert the change here because in some cases a nil attribute value will lead to a specific output that has been defined in YAML mapping rules.

Ah fair. Kinda makes me want a NilPropertyValue instead of magic nil, but this is fine

elsom25 · 2024-03-26T12:28:52Z

app/services/mapping_builder.rb

+      relevant_rules = mapping_rules.select { |rule| rule.input.product_category_id.start_with?(vertical.id) }
+      relevant_rules.map do |rule|
+        {
+          input: rule.input.payload.delete_if { |_k, v| v.nil? },
+          output: rule.output.payload.delete_if { |_k, v| v.nil? },
+        }
+      end


Suggested change

relevant_rules = mapping_rules.select { |rule| rule.input.product_category_id.start_with?(vertical.id) }

relevant_rules.map do |rule|

{

input: rule.input.payload.delete_if { |_k, v| v.nil? },

output: rule.output.payload.delete_if { |_k, v| v.nil? },

}

end

mapping_rules

.select { _1.input.product_category_id.start_with?(vertical.id) }

.map do

{

input: _1.input.payload.delete_if { |_k, v| v.nil? },

output: _1.output.payload.delete_if { |_k, v| v.nil? },

}

end

elsom25 · 2024-03-26T12:31:46Z

app/serializers/dist/json_serializer.rb

+      all_mapping_blocks = []
+      puts "Generating mappings ..."
+      mapping_count = 0
+      Integration.all.each do |integration|
+        [true, false].each do |from_shopify|
+          mapping_rule_block = mapping_rules.where(
+            integration_id: integration.id,
+            from_shopify: from_shopify,
+          )
+          next if mapping_rule_block.count.zero?
+
+          mappings = []
+          Category.verticals.each do |vertical|
+            mappings << MappingBuilder.build_one_to_one_mappings_for_vertical(
+              mapping_rules: mapping_rule_block,
+              vertical: vertical,
+            )
+          end
+          processed_mappings = mappings.flatten.compact
+          mapping_count += processed_mappings.count
+          all_mapping_blocks << {
+            input_taxonomy: "shopify/v1",
+            output_taxonomy: "#{integration.name}/v1",
+            rules: processed_mappings,
+          }
+        end
+      end


I had done this piece by piece, but assembling it:

Suggested change

all_mapping_blocks = []

puts "Generating mappings ..."

mapping_count = 0

Integration.all.each do |integration|

[true, false].each do |from_shopify|

mapping_rule_block = mapping_rules.where(

integration_id: integration.id,

from_shopify: from_shopify,

)

next if mapping_rule_block.count.zero?

mappings = []

Category.verticals.each do |vertical|

mappings << MappingBuilder.build_one_to_one_mappings_for_vertical(

mapping_rules: mapping_rule_block,

vertical: vertical,

)

end

processed_mappings = mappings.flatten.compact

mapping_count += processed_mappings.count

all_mapping_blocks << {

input_taxonomy: "shopify/v1",

output_taxonomy: "#{integration.name}/v1",

rules: processed_mappings,

}

end

end

mapping_rules = Integration.all.flat_map do |integration|

[true, false].filter_map do |from_shopify|

rules = mapping_rules.where(integration:, from_shopify:)

rules if rule.any?

end

end

rules = Category.verticals.flat_map do |vertical|

MappingBuilder.build_one_to_one_mappings_for_vertical(mapping_rules:, vertical:)

end

mapping_rules.map do |mapping|

{

input_taxonomy: "shopify/v1", # isn't this determined by the true/false technically?

output_taxonomy: "#{mapping.integration.name}/v1",

rules:,

}

end

Thanks! Simplified the code based on the suggestion.

chesterbot01 · 2024-03-26T18:44:54Z

can we start with new dist files for mappings only?

Sounds good, moved taxonomy mapping to a separate file for now.

elsom25 · 2024-03-26T21:44:30Z

app/serializers/dist/json_serializer.rb

@@ -2,9 +2,10 @@

 module Dist
  class JSONSerializer
-    def initialize(verticals:, properties:, version:)
+    def initialize(verticals:, properties:, mapping_rules: nil, version:)


Do we need default value here?

Suggested change

def initialize(verticals:, properties:, mapping_rules: nil, version:)

def initialize(verticals:, properties:, mapping_rules:, version:)

elsom25 · 2024-03-26T21:45:15Z

app/serializers/dist/json_serializer.rb

@@ -70,5 +79,56 @@ def serialize_nested(connection)
        name: connection.name,
      }
    end
+
+    def build_mapping_blocks
+      puts "Generating mappings ..."


Do we need this in our serializers? I think it's reasonable for serializers to not have console output 😅

elsom25

Looks good, though I'd like someone from multi-channel land to give this a check before merge as well (cc @jeanrick)

elsom25 · 2024-03-26T21:52:42Z

app/serializers/dist/json_serializer.rb

@@ -70,5 +79,54 @@ def serialize_nested(connection)
        name: connection.name,
      }
    end
+
+    def build_mapping_blocks
+      mapping_count = 0


We don't need this anymore, correct?

Yea, good catch!

elsom25 · 2024-03-28T16:56:34Z

Makefile

@@ -52,6 +54,7 @@ LOCAL_DB = tmp/local.sqlite3
 # CUE imports needed for schema validation
 ATTRIBUTES_DATA_CUE = schema/attributes_data.cue
 CATEGORIES_DATA_CUE = schema/categories_data.cue
+MAPPINGS_DATA_CUE = schema/mappings_data.cue


You only need this if you need the data generated as part of integrity checks (see validations.cue)

without schema/mappings_data.cue, cue vet -c will error out

mappings: field is required but not present: ./schema.cue:59:1

we have some validation logic for mappings defined in schema/schema.cue

jeanrick · 2024-04-03T16:26:13Z

data/integrations/google/2021-09-21/mappings/from_shopify.yml

+input_taxonomy: "2024-04"
+output_taxonomy: "2021-09-21"


We should still prefix these versions with the platform e.g. shopify/2024-04 and google/2021-09-21

Thanks! Done.

…ning

chesterbot01 marked this pull request as ready for review March 26, 2024 02:18

chesterbot01 requested review from elsom25 and jeanrick March 26, 2024 02:18

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from 2781d35 to 8d934f1 Compare March 26, 2024 03:10

elsom25 reviewed Mar 26, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from e99c901 to a886c6a Compare March 26, 2024 21:44

elsom25 reviewed Mar 26, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from a886c6a to c1a3983 Compare March 26, 2024 21:45

elsom25 approved these changes Mar 26, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from c1a3983 to 4bfb869 Compare March 26, 2024 21:47

elsom25 reviewed Mar 26, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch 5 times, most recently from 4ff15f0 to 16fad0a Compare March 28, 2024 15:26

elsom25 reviewed Mar 28, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch 7 times, most recently from 582c246 to 1ee66df Compare April 2, 2024 20:48

jeanrick reviewed Apr 3, 2024

View reviewed changes

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from 1ee66df to 33cdb5d Compare April 3, 2024 17:49

chesterbot01 added 5 commits April 4, 2024 15:59

Generate inline taxonomy mapping in taxonomy.json

077518d

Simpilify the code based on feedbacks

657c353

Move mappings to a separate file under /dist

7ad3af2

Add cue validation for mappings

9e5fbe8

tweak folder structure and mapping models to specifiy taxonomy versio…

4819d48

…ning

chesterbot01 force-pushed the add-google-taxonomy-mapping branch from 33cdb5d to 4819d48 Compare April 4, 2024 22:03

jeanrick approved these changes Apr 5, 2024

View reviewed changes

chesterbot01 merged commit e8ec78a into main Apr 5, 2024
3 checks passed

chesterbot01 deleted the add-google-taxonomy-mapping branch April 5, 2024 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate inline taxonomy mapping in taxonomy.json #123

Generate inline taxonomy mapping in taxonomy.json #123

chesterbot01 commented Mar 26, 2024 •

edited

Loading

elsom25 commented Mar 26, 2024

elsom25 commented Mar 26, 2024

elsom25 Mar 26, 2024

elsom25 Mar 26, 2024

elsom25 Mar 26, 2024

elsom25 Mar 26, 2024

elsom25 Mar 26, 2024

chesterbot01 Mar 28, 2024

elsom25 Mar 28, 2024

elsom25 Mar 26, 2024

elsom25 Mar 26, 2024

chesterbot01 Mar 26, 2024

chesterbot01 commented Mar 26, 2024 •

edited

Loading

elsom25 Mar 26, 2024

chesterbot01 Mar 26, 2024

elsom25 Mar 26, 2024

elsom25 left a comment

elsom25 Mar 26, 2024

chesterbot01 Mar 26, 2024

elsom25 Mar 28, 2024

chesterbot01 Mar 28, 2024 •

edited

Loading

jeanrick Apr 3, 2024

chesterbot01 Apr 3, 2024

		Integration.all.each do \|integration\|
		[true, false].each do \|from_shopify\|

-      Integration.all.each do |integration|
-        [true, false].each do |from_shopify|
+      mapping_rules = Integration.all.flat_map do |integration|
+        [true, false].filter_map do |from_shopify|
+          rules = mapping_rules.where(integration:, from_shopify:)
+          rules if rule.any?
+        end
+      end

	value: attribute[:value].nil? ? nil : PropertyValue.find(attribute[:value]).gid,
	value: attribute[:value] \|\| PropertyValue.find(attribute[:value]).gid,

	def initialize(verticals:, properties:, mapping_rules: nil, version:)
	def initialize(verticals:, properties:, mapping_rules:, version:)

Generate inline taxonomy mapping in taxonomy.json #123

Generate inline taxonomy mapping in taxonomy.json #123

Conversation

chesterbot01 commented Mar 26, 2024 • edited Loading

elsom25 commented Mar 26, 2024

elsom25 commented Mar 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chesterbot01 commented Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elsom25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chesterbot01 Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chesterbot01 commented Mar 26, 2024 •

edited

Loading

chesterbot01 commented Mar 26, 2024 •

edited

Loading

chesterbot01 Mar 28, 2024 •

edited

Loading