From c8607172aebaf0f714e85f93acd9a39738643cc1 Mon Sep 17 00:00:00 2001 From: Roel Bondoc Date: Tue, 20 Oct 2020 17:29:48 -0400 Subject: [PATCH 1/5] Add an irb binding console command This adds an IRB console within the context of robles. It also adds a `book` variable as a convenience for working with the publisher. --- app/commands/robles_cli.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/commands/robles_cli.rb b/app/commands/robles_cli.rb index 2e40147..7e970cb 100644 --- a/app/commands/robles_cli.rb +++ b/app/commands/robles_cli.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'irb' require 'guard' require 'guard/commander' # needed because of https://github.com/guard/guard/issues/793 @@ -32,6 +33,15 @@ def serve RoblesServer.run! end + desc 'console [PUBLISH_FILE]', 'opens an interactive Ruby console' + option :'publish-file', type: :string, desc: 'Location of the publish.yaml file' + def console + publish_file = options.fetch('publish_file', runner.default_publish_file) + parser = Parser::Publish.new(file: publish_file) + book = parser.parse + binding.irb + end + desc 'publish [PUBLISH_FILE]', 'renders and publishes a book' option :'publish-file', type: :string, desc: 'Location of the publish.yaml file' def publish From 1e11d6da915dfe8b07161696fe7ec9b90d3b5ec0 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Tue, 20 Oct 2020 16:43:03 +0100 Subject: [PATCH 2/5] WEB-3823: Allows specification of image variants as attachments This adds the new image types that are required for display across the site. It also changes the upload API, where image URLs are now provided as an array of objects, each with `url` and `variant` keys. --- app/commands/robles_cli.rb | 3 +-- app/lib/image_provider/extractor.rb | 2 +- .../markdown_image_extractor.rb | 6 +++++- app/lib/image_provider/provider.rb | 4 ++-- app/lib/parser/publish.rb | 6 +++--- app/lib/renderer/image_attachable.rb | 3 +-- app/lib/renderer/image_attributes.rb | 2 ++ app/models/book.rb | 10 +++++---- app/models/concerns/image_attachable.rb | 21 +++++++++++++------ app/models/image.rb | 9 ++++---- app/models/image_representation.rb | 19 ++++++++++++++++- 11 files changed, 59 insertions(+), 26 deletions(-) diff --git a/app/commands/robles_cli.rb b/app/commands/robles_cli.rb index 7e970cb..4a9b9b0 100644 --- a/app/commands/robles_cli.rb +++ b/app/commands/robles_cli.rb @@ -16,8 +16,7 @@ def self.exit_on_failure? option :local, type: :boolean def render book = runner.render(publish_file: options['publish_file'], local: options['local']) - p book.sections.first.chapters.last - p book.contributors.to_json + p book.cover_image_url.to_json end desc 'serve', 'starts local preview server' diff --git a/app/lib/image_provider/extractor.rb b/app/lib/image_provider/extractor.rb index 0c39a41..de45ed1 100644 --- a/app/lib/image_provider/extractor.rb +++ b/app/lib/image_provider/extractor.rb @@ -12,7 +12,7 @@ def initialize(book) def extract @images = image_paths.map do |path| - Image.with_representations(local_url: path[:absolute_path], sku: book.sku) + Image.with_representations({ local_url: path[:absolute_path], sku: book.sku }, variants: path[:variants]) end end diff --git a/app/lib/image_provider/markdown_image_extractor.rb b/app/lib/image_provider/markdown_image_extractor.rb index e60b90e..c12c996 100644 --- a/app/lib/image_provider/markdown_image_extractor.rb +++ b/app/lib/image_provider/markdown_image_extractor.rb @@ -18,7 +18,11 @@ def images end def image_record(url) - { relative_path: cleanpath(url), absolute_path: cleanpath(apply_path(url)) } + { + relative_path: cleanpath(url), + absolute_path: cleanpath(apply_path(url)), + variants: ImageRepresentation::DEFAULT_WIDTHS.keys + } end def cleanpath(path) diff --git a/app/lib/image_provider/provider.rb b/app/lib/image_provider/provider.rb index 03b93ef..0b27659 100644 --- a/app/lib/image_provider/provider.rb +++ b/app/lib/image_provider/provider.rb @@ -27,8 +27,8 @@ def process def representations_for_local_url(url) clean_url = Pathname.new(url).cleanpath.to_s extractor.images - .find { |image| image.local_url == clean_url } - &.representations + .filter { |image| image.local_url == clean_url } + .flat_map(&:representations) end def extractor diff --git a/app/lib/parser/publish.rb b/app/lib/parser/publish.rb index 45c7396..5ea4708 100644 --- a/app/lib/parser/publish.rb +++ b/app/lib/parser/publish.rb @@ -7,9 +7,9 @@ class Publish include Linting::FileExistenceChecker VALID_BOOK_ATTRIBUTES = %i[sku edition title description_md released_at materials_url - cover_image gallery_image twitter_card_image trailer_video_url - version_description professional difficulty platform - language editor domains categories who_is_this_for_md + cover_image gallery_image twitter_card_image artwork_image icon_image + trailer_video_url version_description professional difficulty + platform language editor domains categories who_is_this_for_md covered_concepts_md hide_chapter_numbers in_flux forum_url pages short_description recommended_skus isbn amazon_url].freeze diff --git a/app/lib/renderer/image_attachable.rb b/app/lib/renderer/image_attachable.rb index a6fd440..a41a933 100644 --- a/app/lib/renderer/image_attachable.rb +++ b/app/lib/renderer/image_attachable.rb @@ -15,8 +15,7 @@ def attach_images return if image_provider.blank? object.image_attachment_loop do |local_url| - representations = image_provider.representations_for_local_url(local_url) - representations.find { |r| r.width == :original }.remote_url + image_provider.representations_for_local_url(local_url) end end end diff --git a/app/lib/renderer/image_attributes.rb b/app/lib/renderer/image_attributes.rb index 871255b..0ef99c0 100644 --- a/app/lib/renderer/image_attributes.rb +++ b/app/lib/renderer/image_attributes.rb @@ -19,6 +19,8 @@ def src(relative_url) def representations(local_url) image_provider.representations_for_local_url(local_url) + .filter { |r| ImageRepresentation::DEFAULT_WIDTHS.keys.include?(r.variant) } + .uniq(&:variant) end def class_list(alt_text) diff --git a/app/models/book.rb b/app/models/book.rb index 0ef680e..ca0abc4 100644 --- a/app/models/book.rb +++ b/app/models/book.rb @@ -8,15 +8,17 @@ class Book include Concerns::MarkdownRenderable attr_accessor :sku, :edition, :title, :description_md, :released_at, :sections, :git_commit_hash, - :materials_url, :cover_image, :gallery_image, :twitter_card_image, - :trailer_video_url, :version_description, :professional, :difficulty, + :materials_url, :cover_image, :gallery_image, :twitter_card_image, :artwork_image, + :icon_image, :trailer_video_url, :version_description, :professional, :difficulty, :platform, :language, :editor, :domains, :categories, :who_is_this_for_md, :covered_concepts_md, :root_path, :hide_chapter_numbers, :in_flux, :forum_url, :pages, :short_description, :recommended_skus, :contributors, :price_band, :isbn, :amazon_url - attr_image :cover_image_url, source: :cover_image + attr_image :cover_image_url, source: :cover_image, variants: %i[original w594 w300] attr_image :gallery_image_url, source: :gallery_image - attr_image :twitter_card_image_url, source: :twitter_card_image + attr_image :twitter_card_image_url, source: :twitter_card_image, variants: %i[original w1800] + attr_image :artwork_image_url, source: :artwork_image, variants: %i[original w180] + attr_image :icon_image_url, source: :icon_image, variants: %i[original w180] attr_markdown :who_is_this_for, source: :who_is_this_for_md, file: false attr_markdown :covered_concepts, source: :covered_concepts_md, file: false attr_markdown :description, source: :description_md, file: false diff --git a/app/models/concerns/image_attachable.rb b/app/models/concerns/image_attachable.rb index f4729fe..d7c7180 100644 --- a/app/models/concerns/image_attachable.rb +++ b/app/models/concerns/image_attachable.rb @@ -11,8 +11,14 @@ module ImageAttachable class_methods do # Specify the name of the attribute that the CDN image URL should be stored in - def attr_image(attribute, source:) - _image_attachable_attributes.push({ destination: attribute, source: source }) + def attr_image(attribute, source:, variants: nil) + _image_attachable_attributes.push( + { + destination: attribute, + source: source, + variants: variants.presence || %i[original] + } + ) attr_accessor attribute end end @@ -25,12 +31,13 @@ def image_attachment_paths { relative_path: path, - absolute_path: (Pathname.new(root_path) + path).to_s + absolute_path: (Pathname.new(root_path) + path).to_s, + variants: attr[:variants] } end.compact end - # Allows looping through the image attachment attributes, populating their remote URLs + # Allows looping through the image attachment attributes, populating the remote representations # Takes a block with one argument--the local URL of the file def image_attachment_loop(&block) _image_attachable_attributes.each do |attribute| @@ -38,8 +45,10 @@ def image_attachment_loop(&block) next if path.blank? local_url = (Pathname.new(root_path) + path).to_s - remote_url = block.call(local_url) - send("#{attribute[:destination]}=".to_sym, remote_url) + representations = block.call(local_url) + .filter { |r| attribute[:variants].include?(r.variant) } + .uniq(&:variant) + send("#{attribute[:destination]}=".to_sym, representations) end end end diff --git a/app/models/image.rb b/app/models/image.rb index 5bace40..cb410d9 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -8,9 +8,10 @@ class Image attr_accessor :local_url, :representations - def self.with_representations(attributes = {}) + def self.with_representations(attributes = {}, variants: nil) + variants ||= ImageRepresentation::DEFAULT_WIDTHS.keys new(attributes).tap do |image| - image.representations = ImageRepresentation::WIDTHS.keys.map do |width| + image.representations = variants.map do |width| ImageRepresentation.new(width: width, image: image) end end @@ -48,9 +49,9 @@ def upload next end - logger.info "Generating #{representation.width}px for #{local_url}" + logger.info "Generating #{representation.width} variant for #{local_url}" representation.generate - logger.info "Uploading #{representation.width}px for #{local_url}" + logger.info "Uploading #{representation.width} variant for #{local_url}" representation.upload end end diff --git a/app/models/image_representation.rb b/app/models/image_representation.rb index 2b9861b..0dc2677 100644 --- a/app/models/image_representation.rb +++ b/app/models/image_representation.rb @@ -3,16 +3,25 @@ # Defines a specific size of an image class ImageRepresentation include ActiveModel::Model + include ActiveModel::Serializers::JSON include ImageProvider::Concerns::Resizable include ImageProvider::Concerns::Uploadable - WIDTHS = { + DEFAULT_WIDTHS = { small: 150, medium: 300, large: 700, original: nil }.freeze + OTHER_WIDTHS = { + w180: 180, + w300: 300, + w594: 594 + }.freeze + + WIDTHS = DEFAULT_WIDTHS.merge(OTHER_WIDTHS).freeze + attr_accessor :width, :image validates :width, inclusion: { in: WIDTHS.keys }, presence: true @@ -36,4 +45,12 @@ def width_px def source_url image.local_url end + + # Used for serialisation + alias url remote_url + alias variant width + + def attributes + { url: nil, variant: nil } + end end From 513a70acf404e1de7359adeb3f67f34a20cd9e90 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Tue, 20 Oct 2020 17:52:24 +0100 Subject: [PATCH 3/5] WEB-3823: Forgot to add the new attributes to the serialisation hash --- app/models/book.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/models/book.rb b/app/models/book.rb index ca0abc4..2b192c2 100644 --- a/app/models/book.rb +++ b/app/models/book.rb @@ -37,11 +37,12 @@ def initialize(attributes = {}) # Used for serialisation def attributes { sku: nil, edition: nil, title: nil, description: nil, released_at: nil, sections: [], - git_commit_hash: nil, materials_url: nil, cover_image_url: nil, gallery_image_url: nil, - twitter_card_image_url: nil, trailer_video_url: nil, version_description: nil, - professional: nil, difficulty: nil, platform: nil, language: nil, editor: nil, domains: [], - categories: [], who_is_this_for: nil, covered_concepts: nil, hide_chapter_numbers: nil, - in_flux: nil, forum_url: nil, pages: nil, short_description: nil, recommended_skus: [], - contributors: [], price_band: nil, isbn: nil, amazon_url: nil }.stringify_keys + git_commit_hash: nil, materials_url: nil, cover_image_url: [], gallery_image_url: [], + twitter_card_image_url: [], artwork_image_url: [], icon_image_url: [], + trailer_video_url: nil, version_description: nil, professional: nil, difficulty: nil, + platform: nil, language: nil, editor: nil, domains: [], categories: [], who_is_this_for: nil, + covered_concepts: nil, hide_chapter_numbers: nil, in_flux: nil, forum_url: nil, pages: nil, + short_description: nil, recommended_skus: [], contributors: [], price_band: nil, isbn: nil, + amazon_url: nil }.stringify_keys end end From 6636bc1219bb6cd01a5c47bfa426a43b7ff22220 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Tue, 20 Oct 2020 17:45:46 +0100 Subject: [PATCH 4/5] WEB-3827: Adding a new edition attribute to vend.yaml This is intended as a check to ensure that vend.yaml is reviewed before a new release can be published. --- app/lib/linting/linter.rb | 2 +- app/lib/linting/metadata/edition_reference.rb | 6 ++++-- app/lib/linting/vend_linter.rb | 5 +++-- app/lib/parser/vend.rb | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/lib/linting/linter.rb b/app/lib/linting/linter.rb index bbecd86..76b52ae 100644 --- a/app/lib/linting/linter.rb +++ b/app/lib/linting/linter.rb @@ -46,7 +46,7 @@ def lint_with_ui(options:, show_ui: true) # rubocop:disable Metrics/MethodLength if file_exists?(vend_file) with_spinner(title: 'Validating {{bold:vend.yaml}}', show: show_ui) do - annotations.concat(Linting::VendLinter.new(file: vend_file).lint) + annotations.concat(Linting::VendLinter.new(file: vend_file).lint(options: options)) end else puts CLI::UI.fmt('{{x}} Unable to find {{bold:vend.yaml}}--skipping validation.') diff --git a/app/lib/linting/metadata/edition_reference.rb b/app/lib/linting/metadata/edition_reference.rb index 8210721..ce14003 100644 --- a/app/lib/linting/metadata/edition_reference.rb +++ b/app/lib/linting/metadata/edition_reference.rb @@ -34,13 +34,15 @@ def error_annotation locate_edition_reference.merge( absolute_path: file, annotation_level: 'failure', - message: "The edition attribute in `publish.yaml` (#{edition_from_metadata}) should be the same as the one\nspecified in the git branch name (#{current_branch}).", + message: "The edition attribute in #{Pathname.new(file).basename} (#{edition_from_metadata}) should be the same as the one\nspecified in the git branch name (#{current_branch}).", title: 'Invalid edition specified' ) ) end - def locate_edition_reference + def locate_edition_reference # rubocop:disable Metrics/MethodLength + return { start_line: 0, end_line: 0 } unless attributes[:edition].present? + IO.foreach(file).with_index do |line, line_number| next unless line.include?('edition:') diff --git a/app/lib/linting/vend_linter.rb b/app/lib/linting/vend_linter.rb index 21ba6d0..bc95977 100644 --- a/app/lib/linting/vend_linter.rb +++ b/app/lib/linting/vend_linter.rb @@ -3,7 +3,7 @@ module Linting # Lints vend.yaml class VendLinter # rubocop:disable Metrics/ClassLength - VALID_PRICE_BANDS = %w(free 2020_full_book 2020_short_book 2020_deprecated_book).freeze + VALID_PRICE_BANDS = %w[free 2020_full_book 2020_short_book 2020_deprecated_book].freeze attr_reader :file, :vend_file def initialize(file:) @@ -11,12 +11,13 @@ def initialize(file:) @annotations = [] end - def lint + def lint(options: {}) load_file return @annotations unless @annotations.empty? check_price_band check_total_percentage + @annotations.concat(Linting::Metadata::EditionReference.lint(file: file, attributes: vend_file)) unless options['without-edition'] return @annotations unless @annotations.empty? check_for_razeware_user diff --git a/app/lib/parser/vend.rb b/app/lib/parser/vend.rb index 6ff03d6..ee0f812 100644 --- a/app/lib/parser/vend.rb +++ b/app/lib/parser/vend.rb @@ -8,7 +8,8 @@ class Vend def parse { contributors: contributors, - price_band: vend_file[:price_band] + price_band: vend_file[:price_band], + edition: vend_file[:edition] } end From 0ca1c568d32a9c09afd946882451db9448a056d8 Mon Sep 17 00:00:00 2001 From: Sam Davies Date: Fri, 23 Oct 2020 08:41:03 +0100 Subject: [PATCH 5/5] WEB-3823: Forgot that we needed 1800 wide too --- app/models/image_representation.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/image_representation.rb b/app/models/image_representation.rb index 0dc2677..f2443da 100644 --- a/app/models/image_representation.rb +++ b/app/models/image_representation.rb @@ -17,7 +17,8 @@ class ImageRepresentation OTHER_WIDTHS = { w180: 180, w300: 300, - w594: 594 + w594: 594, + w1800: 1800 }.freeze WIDTHS = DEFAULT_WIDTHS.merge(OTHER_WIDTHS).freeze