From 811f812e11f827ee0ba127a2fd462f37e711463e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 10:05:12 -0400 Subject: [PATCH 01/17] Create application asset context **Why**: Asset paths include a fingerprint which can't be known to the client unless provided by the server. Co-authored-by: Will Beddow --- app/assets/javascripts/application.js | 1 + app/assets/javascripts/assets.js.erb | 13 +++++++++++++ .../app/document-capture/context/asset.js | 5 +++++ app/javascript/packs/document-capture.jsx | 7 +++++-- .../app/document-capture/context/asset-spec.jsx | 16 ++++++++++++++++ 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/assets.js.erb create mode 100644 app/javascript/app/document-capture/context/asset.js create mode 100644 spec/javascripts/app/document-capture/context/asset-spec.jsx diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index be141f9340b..d60736db56a 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -1,2 +1,3 @@ //= require i18n-strings +//= require assets //= require local-time diff --git a/app/assets/javascripts/assets.js.erb b/app/assets/javascripts/assets.js.erb new file mode 100644 index 00000000000..f4c170af449 --- /dev/null +++ b/app/assets/javascripts/assets.js.erb @@ -0,0 +1,13 @@ +window.LoginGov = window.LoginGov || {}; +window.LoginGov.assets = {}; + +<% keys = [ + 'state-id-sample-front.jpg', + 'plus.svg', + 'minus.svg', + 'up-carat-thin.svg' +] %> + +<% keys.each do |key| %> + window.LoginGov.assets['<%= ActionController::Base.helpers.j key %>'] = '<%= ActionController::Base.helpers.j ActionController::Base.helpers.asset_path key %>'; +<% end %> diff --git a/app/javascript/app/document-capture/context/asset.js b/app/javascript/app/document-capture/context/asset.js new file mode 100644 index 00000000000..91173cbe6d9 --- /dev/null +++ b/app/javascript/app/document-capture/context/asset.js @@ -0,0 +1,5 @@ +import { createContext } from 'react'; + +const AssetContext = createContext({}); + +export default AssetContext; diff --git a/app/javascript/packs/document-capture.jsx b/app/javascript/packs/document-capture.jsx index 4ee038f0a6a..ef61ee13f84 100644 --- a/app/javascript/packs/document-capture.jsx +++ b/app/javascript/packs/document-capture.jsx @@ -1,10 +1,11 @@ import React from 'react'; import { render } from 'react-dom'; import DocumentCapture from '../app/document-capture/components/document-capture'; +import AssetContext from '../app/document-capture/context/asset'; import I18nContext from '../app/document-capture/context/i18n'; import { Provider as AcuantProvider } from '../app/document-capture/context/acuant'; -const { I18n: i18n } = window.LoginGov; +const { I18n: i18n, assets } = window.LoginGov; function getMetaContent(name) { return document.querySelector(`meta[name="${name}"]`)?.content ?? null; @@ -18,7 +19,9 @@ render( endpoint={getMetaContent('acuant-sdk-initialization-endpoint')} > - + + + , appRoot, diff --git a/spec/javascripts/app/document-capture/context/asset-spec.jsx b/spec/javascripts/app/document-capture/context/asset-spec.jsx new file mode 100644 index 00000000000..670c5696d03 --- /dev/null +++ b/spec/javascripts/app/document-capture/context/asset-spec.jsx @@ -0,0 +1,16 @@ +import React, { useContext } from 'react'; +import { render } from '@testing-library/react'; +import AssetContext from '../../../../../app/javascript/app/document-capture/context/asset'; +import { useDOM } from '../../../support/dom'; + +describe('document-capture/context/asset', () => { + useDOM(); + + const ContextValue = () => JSON.stringify(useContext(AssetContext)); + + it('defaults to empty object', () => { + const { container } = render(); + + expect(container.textContent).to.equal('{}'); + }); +}); From 96e15ce4e08a41b4f701e49aa01f938062108d76 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 10:06:48 -0400 Subject: [PATCH 02/17] Update accordion to check for previous initialization **Why**: Current Login.gov pages will check for and initialize any accordions present at page load. Since React components can be mounted after page load, and also need to initialize, there should be internal checks to protect against double initialization, since otherwise event handlers may be triggered multiple times. --- app/javascript/app/components/accordion.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/javascript/app/components/accordion.js b/app/javascript/app/components/accordion.js index 14f5da6239d..186ddd19985 100644 --- a/app/javascript/app/components/accordion.js +++ b/app/javascript/app/components/accordion.js @@ -17,8 +17,10 @@ class Accordion extends Events { } setup() { - this.bindEvents(); - this.onInitialize(); + if (!this.isInitialized()) { + this.bindEvents(); + this.onInitialize(); + } } bindEvents() { @@ -41,6 +43,11 @@ class Accordion extends Events { onInitialize() { this.setExpanded(false); this.collapsedIcon.classList.remove('display-none'); + this.el.setAttribute('data-initialized', ''); + } + + isInitialized() { + return this.el.hasAttribute('data-initialized'); } handleClick() { From a8ea3cfd44aaaa8053005c1d42e22e24b03526ca Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 10:15:48 -0400 Subject: [PATCH 03/17] Add Image component **Why**: As an abstraction to simply render an image at the known asset path, encapsulating behavior to perform asset lookup by asset context --- .../app/document-capture/components/image.jsx | 27 ++++++++++++ .../components/image-spec.jsx | 41 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 app/javascript/app/document-capture/components/image.jsx create mode 100644 spec/javascripts/app/document-capture/components/image-spec.jsx diff --git a/app/javascript/app/document-capture/components/image.jsx b/app/javascript/app/document-capture/components/image.jsx new file mode 100644 index 00000000000..26a73bdba3c --- /dev/null +++ b/app/javascript/app/document-capture/components/image.jsx @@ -0,0 +1,27 @@ +import React, { useContext } from 'react'; +import PropTypes from 'prop-types'; +import AssetContext from '../context/asset'; + +function Image({ assetPath, alt, ...imgProps }) { + const assets = useContext(AssetContext); + + const src = Object.prototype.hasOwnProperty.call(assets, assetPath) + ? assets[assetPath] + : assetPath; + + // Disable reason: While props spreading can introduce confusion to what is + // being passed down, in this case the component is intended to represent a + // pass-through to a base `` element, with handling for asset paths. + // + // Seee: https://github.com/airbnb/javascript/tree/master/react#props + + // eslint-disable-next-line react/jsx-props-no-spreading + return {alt}; +} + +Image.propTypes = { + assetPath: PropTypes.string.isRequired, + alt: PropTypes.string.isRequired, +}; + +export default Image; diff --git a/spec/javascripts/app/document-capture/components/image-spec.jsx b/spec/javascripts/app/document-capture/components/image-spec.jsx new file mode 100644 index 00000000000..6854f8ecc77 --- /dev/null +++ b/spec/javascripts/app/document-capture/components/image-spec.jsx @@ -0,0 +1,41 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import Image from '../../../../../app/javascript/app/document-capture/components/image'; +import { useDOM } from '../../../support/dom'; +import AssetContext from '../../../../../app/javascript/app/document-capture/context/asset'; + +describe('document-capture/components/image', () => { + useDOM(); + + it('renders the given assetPath as src if the asset is not known', () => { + const { getByAltText } = render( + unknown, + ); + + const img = getByAltText('unknown'); + + expect(img.src).to.equal('unknown.png'); + }); + + it('renders an img at mapped src if known by context', () => { + const { getByAltText } = render( + + icon + , + ); + + const img = getByAltText('icon'); + + expect(img.src).to.equal('icon-12345.png'); + }); + + it('renders with given props', () => { + const { getByAltText } = render( + icon, + ); + + const img = getByAltText('icon'); + + expect(img.width).to.equal(50); + }); +}); From deb624c6685c17004c28699875e21cedbbf98ed7 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 10:17:03 -0400 Subject: [PATCH 04/17] Stub self global in test environment **Why**: If classList.js is imported (directly or indirectly) through any tested files, an error will occur. See inline comment for additional details. --- spec/javascripts/spec_helper.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/javascripts/spec_helper.js b/spec/javascripts/spec_helper.js index 6b37c24633a..df575b7c2ec 100644 --- a/spec/javascripts/spec_helper.js +++ b/spec/javascripts/spec_helper.js @@ -3,3 +3,12 @@ const dirtyChai = require('dirty-chai'); chai.use(dirtyChai); global.expect = chai.expect; + +// classList.js will throw an error when loaded into the test environment, since +// it assumes the presence of a `self` global. This shim is enough only to skip +// the polyfill. It's expected where a DOM is used that JSDOM will provide the +// Element#classList implementation. +// +// See: https://github.com/eligrey/classList.js/issues/48 +// See: https://github.com/eligrey/classList.js/blob/ecb3305/classList.js#L14 +global.self = global; From 5ade1bd588791c152f59e5afc4348e05d21b0e87 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 13:05:12 -0400 Subject: [PATCH 05/17] Add Accordion React component **Why**: To recreate the existing document capture flow, we will need to be able to represent collapsible accordion content in React --- app/assets/javascripts/i18n-strings.js.erb | 5 +- .../document-capture/components/accordion.jsx | 80 +++++++++++++++++++ .../components/accordion-spec.jsx | 24 ++++++ 3 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 app/javascript/app/document-capture/components/accordion.jsx create mode 100644 spec/javascripts/app/document-capture/components/accordion-spec.jsx diff --git a/app/assets/javascripts/i18n-strings.js.erb b/app/assets/javascripts/i18n-strings.js.erb index fefe0ae54a8..da74d1d5d81 100644 --- a/app/assets/javascripts/i18n-strings.js.erb +++ b/app/assets/javascripts/i18n-strings.js.erb @@ -47,7 +47,10 @@ window.LoginGov = window.LoginGov || {}; 'zxcvbn.feedback.for_a_stronger_password_use_a_few_words_separated_by_spaces_but_avoid_common_phrases', 'zxcvbn.feedback.use_a_longer_keyboard_pattern_with_more_turns', 'doc_auth.buttons.take_picture', - 'doc_auth.headings.welcome' + 'doc_auth.headings.welcome', + 'image_description.accordian_plus_buttom', + 'image_description.accordian_minus_buttom', + 'users.personal_key.close' ] %> window.LoginGov.I18n = { diff --git a/app/javascript/app/document-capture/components/accordion.jsx b/app/javascript/app/document-capture/components/accordion.jsx new file mode 100644 index 00000000000..76974a1c0ef --- /dev/null +++ b/app/javascript/app/document-capture/components/accordion.jsx @@ -0,0 +1,80 @@ +import React, { useEffect, useRef, useMemo } from 'react'; +import PropTypes from 'prop-types'; +import BaseAccordion from '../../components/accordion'; +import useI18n from '../hooks/use-i18n'; +import Image from './image'; + +function Accordion({ title, children }) { + const elementRef = useRef(null); + const instanceId = useMemo(() => { + Accordion.instances += 1; + return Accordion.instances; + }, []); + const t = useI18n(); + useEffect(() => { + new BaseAccordion(elementRef.current).setup(); + }, []); + + const contentId = `accordion-content-${instanceId}`; + + return ( +
+
+ +
+ +
+ ); +} + +Accordion.instances = 0; + +Accordion.propTypes = { + title: PropTypes.node.isRequired, + children: PropTypes.node.isRequired, +}; + +export default Accordion; diff --git a/spec/javascripts/app/document-capture/components/accordion-spec.jsx b/spec/javascripts/app/document-capture/components/accordion-spec.jsx new file mode 100644 index 00000000000..258c2e75805 --- /dev/null +++ b/spec/javascripts/app/document-capture/components/accordion-spec.jsx @@ -0,0 +1,24 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import Accordion from '../../../../../app/javascript/app/document-capture/components/accordion'; +import { useDOM } from '../../../support/dom'; + +describe('document-capture/components/accordion', () => { + useDOM(); + + it('renders with a unique ID', () => { + const { container } = render( + <> + Content + Content + , + ); + + const contents = container.querySelectorAll('[id^="accordion-content-"]'); + + expect(contents).to.have.lengthOf(2); + expect(contents[0].id).to.be.ok(); + expect(contents[1].id).to.be.ok(); + expect(contents[0].id).not.to.equal(contents[1].id); + }); +}); From 758e4b3adc40af43f059451797f70b0e0624e02d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 16 Jul 2020 16:53:02 -0400 Subject: [PATCH 06/17] Add document tips accordion to document capture **Why**: Recreating the existing document capture flow requires integrating existing tips content --- app/assets/javascripts/i18n-strings.js.erb | 12 +++++- .../components/document-capture.jsx | 12 ++++++ .../components/document-tips.jsx | 41 +++++++++++++++++++ config/locales/doc_auth/en.yml | 2 + config/locales/doc_auth/es.yml | 2 + config/locales/doc_auth/fr.yml | 2 + 6 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 app/javascript/app/document-capture/components/document-tips.jsx diff --git a/app/assets/javascripts/i18n-strings.js.erb b/app/assets/javascripts/i18n-strings.js.erb index da74d1d5d81..f19f3e3e55d 100644 --- a/app/assets/javascripts/i18n-strings.js.erb +++ b/app/assets/javascripts/i18n-strings.js.erb @@ -50,7 +50,17 @@ window.LoginGov = window.LoginGov || {}; 'doc_auth.headings.welcome', 'image_description.accordian_plus_buttom', 'image_description.accordian_minus_buttom', - 'users.personal_key.close' + 'users.personal_key.close', + 'doc_auth.tips.title', + 'doc_auth.tips.title_more', + 'doc_auth.tips.header_text', + 'doc_auth.tips.text1', + 'doc_auth.tips.text2', + 'doc_auth.tips.text3', + 'doc_auth.tips.text4', + 'doc_auth.tips.text5', + 'doc_auth.tips.text6', + 'doc_auth.tips.text7' ] %> window.LoginGov.I18n = { diff --git a/app/javascript/app/document-capture/components/document-capture.jsx b/app/javascript/app/document-capture/components/document-capture.jsx index 154b3d1d9b0..8da4b8ee12d 100644 --- a/app/javascript/app/document-capture/components/document-capture.jsx +++ b/app/javascript/app/document-capture/components/document-capture.jsx @@ -1,13 +1,25 @@ import React from 'react'; import AcuantCapture from './acuant-capture'; +import DocumentTips from './document-tips'; +import Image from './image'; import useI18n from '../hooks/use-i18n'; function DocumentCapture() { const t = useI18n(); + const sample = ( + Sample front of state issued ID + ); + return ( <>

{t('doc_auth.headings.welcome')}

+ ); diff --git a/app/javascript/app/document-capture/components/document-tips.jsx b/app/javascript/app/document-capture/components/document-tips.jsx new file mode 100644 index 00000000000..07f0fe5c684 --- /dev/null +++ b/app/javascript/app/document-capture/components/document-tips.jsx @@ -0,0 +1,41 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import Accordion from './accordion'; +import useI18n from '../hooks/use-i18n'; + +function DocumentTips({ sample }) { + const t = useI18n(); + + const title = ( + <> + {t('doc_auth.tips.title')} + {` ${t('doc_auth.tips.title_more')}`} + + ); + + return ( + + {t('doc_auth.tips.header_text')} +
    +
  • {t('doc_auth.tips.text1')}
  • +
  • {t('doc_auth.tips.text2')}
  • +
  • {t('doc_auth.tips.text3')}
  • +
  • {t('doc_auth.tips.text4')}
  • +
  • {t('doc_auth.tips.text5')}
  • +
  • {t('doc_auth.tips.text6')}
  • +
  • {t('doc_auth.tips.text7')}
  • +
+ {!!sample &&
{sample}
} +
+ ); +} + +DocumentTips.propTypes = { + sample: PropTypes.node, +}; + +DocumentTips.defaultProps = { + sample: null, +}; + +export default DocumentTips; diff --git a/config/locales/doc_auth/en.yml b/config/locales/doc_auth/en.yml index 29f25e08c0a..4224dd2e1e2 100644 --- a/config/locales/doc_auth/en.yml +++ b/config/locales/doc_auth/en.yml @@ -92,6 +92,8 @@ en: information. text7: Use a high-resolution camera. A good mobile phone or tablet camera will work. + title: "Don't take the photo on a white surface!" + title_more: "See more tips…" title_html: "Don't take the photo on a white surface!    See more tips..." titles: diff --git a/config/locales/doc_auth/es.yml b/config/locales/doc_auth/es.yml index fa31b60c9bf..c6768a299e9 100644 --- a/config/locales/doc_auth/es.yml +++ b/config/locales/doc_auth/es.yml @@ -99,6 +99,8 @@ es: la información. text7: Utilice una cámara de alta resolución. La cámara de un buen teléfono móvil o tableta funcionará. + title: "¡No tome la foto en una superficie blanca!" + title_more: "Ver más…" title_html: "¡No tome la foto en una superficie blanca! Ver más..." titles: doc_auth: Autenticación de documentos diff --git a/config/locales/doc_auth/fr.yml b/config/locales/doc_auth/fr.yml index f4c68e77c47..7e9de68452e 100644 --- a/config/locales/doc_auth/fr.yml +++ b/config/locales/doc_auth/fr.yml @@ -107,6 +107,8 @@ fr: de lire toutes les informations. text7: Utilisez une caméra haute résolution. La caméra d'un bon téléphone mobile ou d'une tablette fonctionnera. + title: "Ne prenez pas la photo sur une surface blanche!" + title_more: "Voir plus…" title_html: "Ne prenez pas la photo sur une surface blanche! Voir plus..." titles: doc_auth: Authentification de document From 878685827444b0a946aff9c00b6bc9a63b079287 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Thu, 16 Jul 2020 14:33:55 -0400 Subject: [PATCH 07/17] Make task to check existence of asset strings --- Makefile | 3 +++ lib/asset_checker.rb | 39 +++++++++++++++++++++++++++++++++++++++ scripts/check-assets | 6 ++++++ 3 files changed, 48 insertions(+) create mode 100644 lib/asset_checker.rb create mode 100755 scripts/check-assets diff --git a/Makefile b/Makefile index 0827b35d1b3..5371b099936 100644 --- a/Makefile +++ b/Makefile @@ -52,5 +52,8 @@ normalize_yaml: i18n-tasks normalize find ./config/locales -type f | xargs ./scripts/normalize-yaml +check_asset_strings: + find ./app/javascript -name "*.js*" | xargs ./scripts/check-assets + generate_deploy_checklist: ruby lib/release_management/generate_deploy_checklist.rb diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb new file mode 100644 index 00000000000..51c16bb3e81 --- /dev/null +++ b/lib/asset_checker.rb @@ -0,0 +1,39 @@ +class AssetChecker + def self.run(argv) + assets_file = 'app/assets/javascripts/assets.js.erb' + translations_file = 'app/assets/javascripts/i18n-strings.js.erb' + @asset_strings = load_included_strings(assets_file) + @translation_strings = load_included_strings(translations_file) + argv.map { |f| check_file(f) } + end + + def self.check_file(file) + data = File.open(file).read + missing_translations = find_missing(data,/\Wt\(['"](.*)['"]\)/, @translation_strings) + missing_assets = find_missing(data, /\WassetPath=["'](.*)['"]/, @asset_strings) + if missing_translations.any? || missing_assets.any? + warn file + missing_translations.each do |t| + warn "Missing translation, #{t}" + end + missing_assets.each do |a| + warn "Missing asset, #{a}" + end + end + end + + def self.find_missing(file_data, pattern, source) + missing = [] + strings = file_data.scan pattern + strings.each do |s| + missing.push(s) unless source.include? s + end + missing + end + + def self.load_included_strings(file) + data = File.open(file).read + key_data = data.split('<% keys = [')[1].split('] %>')[0] + key_data.scan(/['"](.*)['"]/) + end +end diff --git a/scripts/check-assets b/scripts/check-assets new file mode 100755 index 00000000000..0308e9cefd9 --- /dev/null +++ b/scripts/check-assets @@ -0,0 +1,6 @@ +#!/usr/bin/env ruby +$LOAD_PATH.unshift(File.expand_path('../lib', File.dirname(__FILE__))) +require 'asset_checker' + +AssetChecker.run(ARGV) + From ecc0087b1afec358085d243f0d673d3c1f3730cb Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Thu, 16 Jul 2020 14:46:44 -0400 Subject: [PATCH 08/17] Rubocop fixes --- lib/asset_checker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb index 51c16bb3e81..d8a34418fcc 100644 --- a/lib/asset_checker.rb +++ b/lib/asset_checker.rb @@ -9,9 +9,9 @@ def self.run(argv) def self.check_file(file) data = File.open(file).read - missing_translations = find_missing(data,/\Wt\(['"](.*)['"]\)/, @translation_strings) + missing_translations = find_missing(data, /\Wt\(['"](.*)['"]\)/, @translation_strings) missing_assets = find_missing(data, /\WassetPath=["'](.*)['"]/, @asset_strings) - if missing_translations.any? || missing_assets.any? + if missing_translations.any? || missing_assets.any? # rubocop:disable Style/GuardClause warn file missing_translations.each do |t| warn "Missing translation, #{t}" From b14cf5f422345455e783e4751b29ccfa4180f45c Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Thu, 16 Jul 2020 16:37:46 -0400 Subject: [PATCH 09/17] Fix YAML --- config/locales/doc_auth/en.yml | 4 ++-- config/locales/doc_auth/es.yml | 2 +- config/locales/doc_auth/fr.yml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/config/locales/doc_auth/en.yml b/config/locales/doc_auth/en.yml index 4224dd2e1e2..8682fd90f0e 100644 --- a/config/locales/doc_auth/en.yml +++ b/config/locales/doc_auth/en.yml @@ -92,9 +92,9 @@ en: information. text7: Use a high-resolution camera. A good mobile phone or tablet camera will work. - title: "Don't take the photo on a white surface!" - title_more: "See more tips…" + title: Don't take the photo on a white surface! title_html: "Don't take the photo on a white surface!    See more tips..." + title_more: See more tips… titles: doc_auth: Document Authentication diff --git a/config/locales/doc_auth/es.yml b/config/locales/doc_auth/es.yml index c6768a299e9..8f4b8363858 100644 --- a/config/locales/doc_auth/es.yml +++ b/config/locales/doc_auth/es.yml @@ -100,7 +100,7 @@ es: text7: Utilice una cámara de alta resolución. La cámara de un buen teléfono móvil o tableta funcionará. title: "¡No tome la foto en una superficie blanca!" - title_more: "Ver más…" title_html: "¡No tome la foto en una superficie blanca! Ver más..." + title_more: Ver más… titles: doc_auth: Autenticación de documentos diff --git a/config/locales/doc_auth/fr.yml b/config/locales/doc_auth/fr.yml index 7e9de68452e..5f5e92e403b 100644 --- a/config/locales/doc_auth/fr.yml +++ b/config/locales/doc_auth/fr.yml @@ -107,8 +107,8 @@ fr: de lire toutes les informations. text7: Utilisez une caméra haute résolution. La caméra d'un bon téléphone mobile ou d'une tablette fonctionnera. - title: "Ne prenez pas la photo sur une surface blanche!" - title_more: "Voir plus…" + title: Ne prenez pas la photo sur une surface blanche! title_html: "Ne prenez pas la photo sur une surface blanche! Voir plus..." + title_more: Voir plus… titles: doc_auth: Authentification de document From c810130c664499f047f55dc04e268c79209320e4 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Fri, 17 Jul 2020 09:54:32 -0400 Subject: [PATCH 10/17] Review changes --- lib/asset_checker.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb index d8a34418fcc..12e31d86204 100644 --- a/lib/asset_checker.rb +++ b/lib/asset_checker.rb @@ -4,14 +4,17 @@ def self.run(argv) translations_file = 'app/assets/javascripts/i18n-strings.js.erb' @asset_strings = load_included_strings(assets_file) @translation_strings = load_included_strings(translations_file) - argv.map { |f| check_file(f) } + exit_code = (argv.map { |f| check_file(f) }).any? ? 1 : 0 + exit exit_code # rubocop:disable Rails/Exit end def self.check_file(file) data = File.open(file).read - missing_translations = find_missing(data, /\Wt\(['"](.*)['"]\)/, @translation_strings) + missing_found = false + missing_translations = find_missing(data, /\Wt\s?\(['"]([^'^"]*)['"]\)/, @translation_strings) missing_assets = find_missing(data, /\WassetPath=["'](.*)['"]/, @asset_strings) - if missing_translations.any? || missing_assets.any? # rubocop:disable Style/GuardClause + if missing_translations.any? || missing_assets.any? + missing_found = true warn file missing_translations.each do |t| warn "Missing translation, #{t}" @@ -20,15 +23,12 @@ def self.check_file(file) warn "Missing asset, #{a}" end end + missing_found end def self.find_missing(file_data, pattern, source) - missing = [] strings = file_data.scan pattern - strings.each do |s| - missing.push(s) unless source.include? s - end - missing + strings.reject { |s| source.include? s } end def self.load_included_strings(file) From ad88fe6d0bbbf6d40281a7cffbc4fe13eb7c77d9 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Tue, 21 Jul 2020 14:38:32 -0400 Subject: [PATCH 11/17] More review changes --- lib/asset_checker.rb | 14 ++++++-------- scripts/check-assets | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb index 12e31d86204..39a2ff47251 100644 --- a/lib/asset_checker.rb +++ b/lib/asset_checker.rb @@ -1,20 +1,18 @@ class AssetChecker - def self.run(argv) + def self.check_files_for_missing(argv) assets_file = 'app/assets/javascripts/assets.js.erb' translations_file = 'app/assets/javascripts/i18n-strings.js.erb' @asset_strings = load_included_strings(assets_file) @translation_strings = load_included_strings(translations_file) - exit_code = (argv.map { |f| check_file(f) }).any? ? 1 : 0 - exit exit_code # rubocop:disable Rails/Exit + argv.any? { |f| missing_strings?(f) } ? 1 : 0 end - def self.check_file(file) + def self.missing_strings?(file) data = File.open(file).read - missing_found = false missing_translations = find_missing(data, /\Wt\s?\(['"]([^'^"]*)['"]\)/, @translation_strings) missing_assets = find_missing(data, /\WassetPath=["'](.*)['"]/, @asset_strings) - if missing_translations.any? || missing_assets.any? - missing_found = true + has_missing = (missing_translations.any? || missing_assets.any?) + if has_missing warn file missing_translations.each do |t| warn "Missing translation, #{t}" @@ -23,7 +21,7 @@ def self.check_file(file) warn "Missing asset, #{a}" end end - missing_found + has_missing end def self.find_missing(file_data, pattern, source) diff --git a/scripts/check-assets b/scripts/check-assets index 0308e9cefd9..1196355b1d8 100755 --- a/scripts/check-assets +++ b/scripts/check-assets @@ -2,5 +2,5 @@ $LOAD_PATH.unshift(File.expand_path('../lib', File.dirname(__FILE__))) require 'asset_checker' -AssetChecker.run(ARGV) +exit AssetChecker.check_files_for_missing(ARGV) From 875e0b51f5f733e4cedd0b000ca684886dcacf7a Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Tue, 21 Jul 2020 14:54:44 -0400 Subject: [PATCH 12/17] Linter fix --- spec/javascripts/spec_helper.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/javascripts/spec_helper.js b/spec/javascripts/spec_helper.js index d31e6da3e22..dde6239ebe5 100644 --- a/spec/javascripts/spec_helper.js +++ b/spec/javascripts/spec_helper.js @@ -18,4 +18,3 @@ beforeEach(() => { document.body.removeChild(document.body.firstChild); } }); - From ed6f631bb0f16ffe6ff6ed1e794385abee3ecc11 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Tue, 21 Jul 2020 15:00:43 -0400 Subject: [PATCH 13/17] Move exit code to check-assets --- lib/asset_checker.rb | 6 +++--- scripts/check-assets | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb index 39a2ff47251..094932d671c 100644 --- a/lib/asset_checker.rb +++ b/lib/asset_checker.rb @@ -1,13 +1,13 @@ class AssetChecker - def self.check_files_for_missing(argv) + def self.check_files(argv) assets_file = 'app/assets/javascripts/assets.js.erb' translations_file = 'app/assets/javascripts/i18n-strings.js.erb' @asset_strings = load_included_strings(assets_file) @translation_strings = load_included_strings(translations_file) - argv.any? { |f| missing_strings?(f) } ? 1 : 0 + argv.any? { |f| file_has_missing?(f) } end - def self.missing_strings?(file) + def self.file_has_missing?(file) data = File.open(file).read missing_translations = find_missing(data, /\Wt\s?\(['"]([^'^"]*)['"]\)/, @translation_strings) missing_assets = find_missing(data, /\WassetPath=["'](.*)['"]/, @asset_strings) diff --git a/scripts/check-assets b/scripts/check-assets index 1196355b1d8..255e6e76b19 100755 --- a/scripts/check-assets +++ b/scripts/check-assets @@ -2,5 +2,5 @@ $LOAD_PATH.unshift(File.expand_path('../lib', File.dirname(__FILE__))) require 'asset_checker' -exit AssetChecker.check_files_for_missing(ARGV) +exit(AssetChecker.check_files(ARGV) ? 1 : 0) From d57af47e9e6034a3093bd0e8b2fe7e4a57a606ff Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Wed, 22 Jul 2020 10:12:21 -0400 Subject: [PATCH 14/17] Add tests for asset task, and flatten strings for consistency --- lib/asset_checker.rb | 4 +- spec/lib/asset_checker_spec.rb | 91 ++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 spec/lib/asset_checker_spec.rb diff --git a/lib/asset_checker.rb b/lib/asset_checker.rb index 094932d671c..74e7cb78b48 100644 --- a/lib/asset_checker.rb +++ b/lib/asset_checker.rb @@ -25,13 +25,13 @@ def self.file_has_missing?(file) end def self.find_missing(file_data, pattern, source) - strings = file_data.scan pattern + strings = (file_data.scan pattern).flatten strings.reject { |s| source.include? s } end def self.load_included_strings(file) data = File.open(file).read key_data = data.split('<% keys = [')[1].split('] %>')[0] - key_data.scan(/['"](.*)['"]/) + key_data.scan(/['"](.*)['"]/).flatten end end diff --git a/spec/lib/asset_checker_spec.rb b/spec/lib/asset_checker_spec.rb new file mode 100644 index 00000000000..3aafa1fa70f --- /dev/null +++ b/spec/lib/asset_checker_spec.rb @@ -0,0 +1,91 @@ +require 'asset_checker' + +def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translation') + """ + import React from 'react'; + import AcuantCapture from './acuant-capture'; + import DocumentTips from './document-tips'; + import Image from './image'; + import useI18n from '../hooks/use-i18n'; + + function DocumentCapture() { + const t = useI18n(); + + const sample = ( + \"Sample + ); + + return ( + <> +

{t('#{translation}')}

+ + + + ); + } + + export default DocumentCapture; + """ +end + +RSpec.describe AssetChecker do + describe '.check_files' do + + + + let(:translation_strings) { %w[first_translation second_translation] } + let(:asset_strings) { %w[first_asset.png second_asset.gif] } + + context 'with matching assets' do + + let(:tempfile) { Tempfile.new } + + before do + File.open(tempfile.path, 'w') do |f| + f.puts(get_js_with_strings) + end + end + + after { tempfile.unlink } + + it 'identifies no issues ' do + allow(AssetChecker).to receive(:load_included_strings).and_return(asset_strings, + translation_strings) + + expect(AssetChecker.check_files([tempfile.path])).to eq(false) + end + end + + context 'with an asset mismatch' do + + let(:tempfile) { Tempfile.new } + + before do + File.open(tempfile.path, 'w') do |f| + f.puts(get_js_with_strings(asset = 'wont_find.svg', translation = 'not-found')) + end + end + + after { tempfile.unlink } + + it 'identifies issues' do + allow(AssetChecker).to receive(:load_included_strings).and_return(asset_strings, + translation_strings) + expect(AssetChecker).to receive(:warn).with(tempfile.path) + expect(AssetChecker).to receive(:warn).with('Missing translation, not-found') + expect(AssetChecker).to receive(:warn).with('Missing asset, wont_find.svg') + expect(AssetChecker.check_files([tempfile.path])).to eq(true) + end + end + + end + + describe '.load_included_strings' do + + end +end From d0a7477a95015780d18dc879db0638b3f22fc1e9 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Wed, 22 Jul 2020 15:44:54 -0400 Subject: [PATCH 15/17] Linter fixes --- spec/lib/asset_checker_spec.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/spec/lib/asset_checker_spec.rb b/spec/lib/asset_checker_spec.rb index 3aafa1fa70f..bcb5660ab20 100644 --- a/spec/lib/asset_checker_spec.rb +++ b/spec/lib/asset_checker_spec.rb @@ -1,7 +1,7 @@ require 'asset_checker' def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translation') - """ + " import React from 'react'; import AcuantCapture from './acuant-capture'; import DocumentTips from './document-tips'; @@ -30,19 +30,16 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat } export default DocumentCapture; - """ + " end RSpec.describe AssetChecker do describe '.check_files' do - - let(:translation_strings) { %w[first_translation second_translation] } let(:asset_strings) { %w[first_asset.png second_asset.gif] } context 'with matching assets' do - let(:tempfile) { Tempfile.new } before do @@ -62,7 +59,6 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat end context 'with an asset mismatch' do - let(:tempfile) { Tempfile.new } before do @@ -82,10 +78,5 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat expect(AssetChecker.check_files([tempfile.path])).to eq(true) end end - - end - - describe '.load_included_strings' do - end end From 256f2fe895169d1c5004d9c5a7ecc817b9aeb072 Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Wed, 22 Jul 2020 16:43:57 -0400 Subject: [PATCH 16/17] More linter fixes --- spec/lib/asset_checker_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/lib/asset_checker_spec.rb b/spec/lib/asset_checker_spec.rb index bcb5660ab20..e77a818219f 100644 --- a/spec/lib/asset_checker_spec.rb +++ b/spec/lib/asset_checker_spec.rb @@ -1,6 +1,6 @@ require 'asset_checker' -def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translation') +def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translation') # rubocop:disable Metrics/LineLength Lint/UselessAssignment " import React from 'react'; import AcuantCapture from './acuant-capture'; @@ -35,7 +35,6 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat RSpec.describe AssetChecker do describe '.check_files' do - let(:translation_strings) { %w[first_translation second_translation] } let(:asset_strings) { %w[first_asset.png second_asset.gif] } From eb4062e2c92e1d7d5e8144d3efd634791ef02e5d Mon Sep 17 00:00:00 2001 From: Will Beddow Date: Wed, 22 Jul 2020 16:54:11 -0400 Subject: [PATCH 17/17] Change syntax to appease linter --- spec/lib/asset_checker_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/asset_checker_spec.rb b/spec/lib/asset_checker_spec.rb index e77a818219f..8c9b2f5647f 100644 --- a/spec/lib/asset_checker_spec.rb +++ b/spec/lib/asset_checker_spec.rb @@ -1,6 +1,6 @@ require 'asset_checker' -def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translation') # rubocop:disable Metrics/LineLength Lint/UselessAssignment +def get_js_with_strings(asset, translation) " import React from 'react'; import AcuantCapture from './acuant-capture'; @@ -43,7 +43,7 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat before do File.open(tempfile.path, 'w') do |f| - f.puts(get_js_with_strings) + f.puts(get_js_with_strings('first_asset.png', 'first_translation')) end end @@ -62,7 +62,7 @@ def get_js_with_strings(asset = 'first_asset.png', translation = 'first_translat before do File.open(tempfile.path, 'w') do |f| - f.puts(get_js_with_strings(asset = 'wont_find.svg', translation = 'not-found')) + f.puts(get_js_with_strings('wont_find.svg', 'not-found')) end end