Merge branch 'release/0.18.2'

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Change Log
 All notable changes to this project will be documented in this file.
 
+## 0.18.2 - 2021-01-04
+### Changed
+- Improved flexibility of the `HasMetafields` concern
+
 ## 0.18.1 - 2020-04-21
 ### Added
 - Support for Shopify Flow trigger usage monitoring

README.md

@@ -972,7 +972,8 @@ end
 If you're writing resource metafields for your models via the Shopify API, you
 can include `DiscoApp::Concerns::HasMetafields` to gain access to a convenient
 `write_metafields` method. Just make sure that `SHOPIFY_API_CLASS` is defined
-on your class and away you go:
+on your class, that you're calling the method in an API context, and away you
+go:
 
 ```
 class Product < ActiveRecord::Base
@@ -982,18 +983,50 @@ class Product < ActiveRecord::Base
 
 end
 
-@product = Product.find(12345678)
-@product.write_metafields(
-  namespace1: {
-    key1: 'value1',
-    key2: 'value2
-  },
-  namespace2: {
-    key3: 'value3'
-  }
-)
+product = Product.find(12345678)
+
+@shop.with_api_context do
+  product.write_metafields(
+    namespace1: {
+      key1: 'value1',
+      key2: 'value2
+    },
+    namespace2: {
+      key3: 'value3'
+    }
+  )
+end
+```
+
+This also works for shop metafields, although be aware that in this case each
+metafield requires an individual API call:
+
+```
+class DiscoApp::Shop < ActiveRecord::Base
+  include DiscoApp::Concerns::HasMetafields
+
+  SHOPIFY_API_CLASS = ShopifyAPI::Shop
+
+end
+
+# this works, but results in 3 API calls
+@shop.with_api_context do
+  @shop.write_metafields(
+    namespace1: {
+      key1: 'value1',
+      key2: 'value2
+    },
+    namespace2: {
+      key3: 'value3'
+    }
+  )
+end
 ```
 
+Note also that `write_metafields` includes an API call out to fetch any existing
+metafields for the object, so that it can avoid any namespace and key conflicts
+by updating existing metafields rather than trying to create new ones.
+
 ### Rubocop
 DiscoApp adds support for Rubocop and Codeclimate. the .rubocop.yml contains the
 configuration you can tweak to suits your coding style, by enabling/disabling

UPGRADING.md

@@ -3,6 +3,15 @@ This file contains more detailed instructions on what's required when updating
 an application between one release version of the gem to the next. It's intended
 as more in-depth accompaniment to the notes in `CHANGELOG.md` for each version.
 
+## Upgrading from 0.18.1 to 0.18.2
+The `HasMetafields` concern has been upgraded - it now handles updating existing
+metafields, and can also properly support writing shop metafields. In order to
+do this, the `write_metafields` method now involves fetching existing metafields
+from the relevant resource - you may need to account for this in tests. A large
+number of Disco projects had their own implementation of the `HasMetafields`
+concern to handle the update existing case - you may want to check and see if
+you can now remove that from your application code.
+
 ## Upgrading from 0.18.0 to 0.18.1
 Ensure new Shopify Flow Trigger Usage database migrations are brought across and run:
 

VERSION

@@ -1 +1 @@
-0.18.1
+0.18.2

app/models/disco_app/concerns/has_metafields.rb

@@ -1,47 +1,104 @@
-module DiscoApp::Concerns::HasMetafields
-
-  extend ActiveSupport::Concern
-
-  included do
-    # Write multiple metafields for this object in a single call.
-    #
-    # Expects an argument in a nested hash structure with :namespace => :key =>
-    # :value, eg:
-    #
-    #   Product.write_metafields(myapp: {
-    #      key1: 'value1',
-    #      key2: 'value2'
-    #   })
-    #
-    # This method assumes that it is being called within a valid Shopify API
-    # session context, eg @shop.with_api_context { ... }.
-    #
-    # It also assumes that the including class has defined the appropriate value
-    # for SHOPIFY_API_CLASS and that calling the `id` method on the instance
-    # will return the relevant object's Shopify ID.
-    #
-    # Returns true on success, false otherwise.
-    def write_metafields(metafields)
-      self.class::SHOPIFY_API_CLASS.new(
-        id: id,
-        metafields: build_metafields(metafields)
-      ).save
-    end
+module DiscoApp
+  module Concerns
+    module HasMetafields
+
+      extend ActiveSupport::Concern
+
+      included do
+        # Write multiple metafields for this object in a single call.
+        #
+        # Expects an argument in a nested hash structure with :namespace => :key => :value, eg:
+        #
+        #   Product.write_metafields(myapp: {
+        #     key1: 'value1',
+        #     key2: 3,
+        #     key3: { 'value_key' => 'value_value' }
+        #   })
+        #
+        # String, integer and hash values will have their value type detected and set accordingly.
+        #
+        # This method assumes that it is being called within a valid Shopify API session context,
+        # eg @shop.with_api_context { ... }.
+        #
+        # It also assumes that the including class has defined the appropriate value for
+        # SHOPIFY_API_CLASS.
+        #
+        # To avoid an issue with trying to set metafield values for namespace and key pairs that
+        # already exist, this method also performs a lookup of existing metafields as part of the
+        # write process and ensures we set the corresponding metafield ID on the update call if
+        # needed.
+        #
+        # Returns true on success, raises an exception otherwise.
+        def write_metafields(metafields)
+          return write_shop_metafields(metafields) if shopify_api_class_is_shop?
+
+          write_resource_metafields(metafields)
+        end
+
+        # Writing shop metafields is a special case - they need to be saved one by one.
+        def write_shop_metafields(metafields)
+          build_metafields(metafields).all?(&:save!)
+        end
+
+        # Writing regular resource metafields can be done in a single request.
+        def write_resource_metafields(metafields)
+          self.class::SHOPIFY_API_CLASS.new(
+            id: id,
+            metafields: build_metafields(metafields)
+          ).save!
+        end
+
+        # Give a nested hash of metafields in the format described above, return an array of
+        # corresponding ShopifyAPI::Metafield instances.
+        def build_metafields(metafields)
+          metafields.flat_map do |namespace, keys|
+            keys.map do |key, value|
+              ShopifyAPI::Metafield.new(
+                id: existing_metafield_id(namespace, key),
+                namespace: namespace,
+                key: key,
+                value: build_value(value),
+                value_type: build_value_type(value)
+              )
+            end
+          end
+        end
+
+        # Return the metafield value based on the provided value.
+        def build_value(value)
+          return value.to_json if value.is_a?(Hash)
+
+          value
+        end
+
+        # Return the metafield type based on the provided value type.
+        def build_value_type(value)
+          return :json_string if value.is_a?(Hash)
+          return :integer if value.is_a?(Integer)
+
+          :string
+        end
+
+        # Given a namespace and key, attempt to find the ID of a corresponding metafield in the
+        # given set of existing metafields.
+        def existing_metafield_id(namespace, key)
+          existing_metafields.find do |existing_metafield|
+            (existing_metafield.namespace.to_sym == namespace.to_sym) && (existing_metafield.key.to_sym == key.to_sym)
+          end&.id
+        end
+
+        # Fetch and cache existing metafields for this object from the Shopify API.
+        def existing_metafields
+          @existing_metafields ||= begin
+            self.class::SHOPIFY_API_CLASS.new(id: id).metafields
+          end
+        end
+
+        def shopify_api_class_is_shop?
+          self.class::SHOPIFY_API_CLASS == ShopifyAPI::Shop
+        end
+      end
 
-    # Give a nested hash of metafields in the format described above, return
-    # an array of corresponding ShopifyAPI::Metafield instances.
-    def build_metafields(metafields)
-      metafields.map do |namespace, keys|
-        keys.map do |key, value|
-          ShopifyAPI::Metafield.new(
-            namespace: namespace,
-            key: key,
-            value: value,
-            value_type: value.is_a?(Integer) ? :integer : :string
-          )
-        end
-      end.flatten
     end
   end
-
 end

app/models/disco_app/concerns/taggable.rb

@@ -3,7 +3,7 @@ module DiscoApp::Concerns::Taggable
   extend ActiveSupport::Concern
 
   def tags
-    data[:tags].split(',').map(&:strip)
+    data[:tags].to_s.split(',').map(&:strip)
   end
 
   def add_tag(tag)

initialise.sh

@@ -5,7 +5,7 @@ APP_NAME="$1"
 RAILS_VERSION="${RAILS_VERSION:-6.0.2}"
 RUBY_VERSION="${RUBY_VERSION:-2.6.5}"
 NODE_VERSION="${NODE_VERSION:-13.7.0}"
-DISCO_APP_VERSION="${DISCO_APP_VERSION:-0.18.1}"
+DISCO_APP_VERSION="${DISCO_APP_VERSION:-0.18.2}"
 
 if [ -z $APP_NAME ]; then
   echo "Usage: ./initialise.sh app_name (rails_version) (ruby_version) (disco_app_version)"

lib/disco_app/version.rb

@@ -1,5 +1,5 @@
 module DiscoApp
 
-  VERSION = '0.18.1'.freeze
+  VERSION = '0.18.2'.freeze
 
 end

lib/generators/disco_app/install/templates/root/.gitignore

@@ -2,8 +2,10 @@
 *.pgdump
 capybara-*.html
 .rspec
-/log
-/tmp
+/log/*
+/tmp/*
+!/log/.keep
+!/tmp/.keep
 /db/*.sqlite3
 /db/*.sqlite3-journal
 /public/system
@@ -17,6 +19,7 @@ pickle-email-*.html
 .disco_app
 /public/packs
 /public/packs-test
+/public/assets
 /node_modules
 yarn-debug.log*
 .yarn-integrity
@@ -29,6 +32,18 @@ yarn-debug.log*
 .envrc
 .pryrc
 
+# Ignore pidfiles, but keep the directory.
+/tmp/pids/*
+!/tmp/pids/
+!/tmp/pids/.keep
+
+# Ignore uploaded files in development.
+/storage/*
+!/storage/.keep
+
+# Ignore master key for decrypting credentials and more.
+/config/master.key
+
 # these should all be checked in to normalise the environment:
 # Gemfile.lock, .ruby-version, .ruby-gemset
 
@@ -37,6 +52,7 @@ yarn-debug.log*
 
 # if using bower-rails ignore default bower_components path bower.json files
 /vendor/assets/bower_components
+
 *.bowerrc
 bower.json
 

test/dummy/app/models/disco_app/shop.rb

@@ -4,6 +4,9 @@ class DiscoApp::Shop < ApplicationRecord
 
   include DiscoApp::Concerns::Shop
 
+  include DiscoApp::Concerns::HasMetafields
+  SHOPIFY_API_CLASS = ShopifyAPI::Shop
+
   has_one :js_configuration
   has_one :widget_configuration
   has_many :carts

test/fixtures/api/widget_store/products/get_metafields_empty_response.json

@@ -0,0 +1,3 @@
+{
+  "metafields": []
+}

test/fixtures/api/widget_store/products/get_metafields_with_existing_response.json

@@ -0,0 +1,11 @@
+{
+  "metafields": [
+    {
+      "id": 874857827343,
+      "namespace": "namespace1",
+      "key": "key1",
+      "value": "value9",
+      "value_type": "string"
+    }
+  ]
+}

test/fixtures/api/widget_store/products/write_metafields_multiple_namespaces_request.json

@@ -3,24 +3,28 @@
     "id": 632910393,
     "metafields": [
       {
+        "id": null,
         "namespace": "namespace1",
         "key": "n1key1",
         "value": "value1",
         "value_type": "string"
       },
       {
+        "id": null,
         "namespace": "namespace1",
         "key": "n1key2",
         "value": 2,
         "value_type": "integer"
       },
       {
+        "id": null,
         "namespace": "namespace2",
         "key": "n2key3",
         "value": "value3",
         "value_type": "string"
       },
       {
+        "id": null,
         "namespace": "namespace2",
         "key": "n2key4",
         "value": 2,

test/fixtures/api/widget_store/products/write_metafields_single_namespace_request.json

@@ -3,12 +3,14 @@
     "id": 632910393,
     "metafields": [
       {
+        "id": null,
         "namespace": "namespace1",
         "key": "key1",
         "value": "value1",
         "value_type": "string"
       },
       {
+        "id": null,
         "namespace": "namespace1",
         "key": "key2",
         "value": 2,