From 68e94f524baaeb85747bbca57ae23308af5ac0f2 Mon Sep 17 00:00:00 2001
From: Nick Taylor <nick@iamdeveloper.com>
Date: Thu, 24 Oct 2019 17:27:16 -0400
Subject: [PATCH] Refactored to use a common <OrganizationPicker /> component
 (#4470)

* Refactored to use a common <OrganizationPicker /> component.

* Small refactor to organization <option />s.

* Renamed files from OrgSettings* to OrganizationPicker*

* Brought orgsettings inline to article form.

* Brought orgsettings inline to listings form.

* Rename onBlur to onToggle for org picker instances. Ooops.

* Updated <OrganizationPicker /> test.
---
 app/javascript/article-form/articleForm.jsx   | 25 ++++----
 .../article-form/elements/orgSettings.jsx     | 51 ---------------
 .../listings/elements/orgSettings.jsx         | 55 ----------------
 app/javascript/listings/listingForm.jsx       | 24 ++++---
 .../organization/OrganizationPicker.jsx       | 46 +++++++++++++
 .../__tests__/OrganizationPicker.test.jsx     | 49 ++++++++++++++
 .../OrganizationPicker.test.jsx.snap          | 64 +++++++++++++++++++
 .../src/components/common-prop-types/index.js |  1 +
 .../organization-prop-type.js                 |  6 ++
 9 files changed, 195 insertions(+), 126 deletions(-)
 delete mode 100644 app/javascript/article-form/elements/orgSettings.jsx
 delete mode 100644 app/javascript/listings/elements/orgSettings.jsx
 create mode 100644 app/javascript/organization/OrganizationPicker.jsx
 create mode 100644 app/javascript/organization/__tests__/OrganizationPicker.test.jsx
 create mode 100644 app/javascript/organization/__tests__/__snapshots__/OrganizationPicker.test.jsx.snap
 create mode 100644 app/javascript/src/components/common-prop-types/organization-prop-type.js

diff --git a/app/javascript/article-form/articleForm.jsx b/app/javascript/article-form/articleForm.jsx
index fcf539acc..4faec2b5d 100644
--- a/app/javascript/article-form/articleForm.jsx
+++ b/app/javascript/article-form/articleForm.jsx
@@ -16,10 +16,10 @@ import Title from './elements/title';
 import MainImage from './elements/mainImage';
 import ImageManagement from './elements/imageManagement';
 import MoreConfig from './elements/moreConfig';
-import OrgSettings from './elements/orgSettings';
 import Errors from './elements/errors';
 import KeyboardShortcutsHandler from './elements/keyboardShortcutsHandler';
 import Tags from '../shared/components/tags';
+import { OrganizationPicker } from '../organization/OrganizationPicker';
 
 const SetupImageButton = ({
   className,
@@ -88,7 +88,7 @@ export default class ArticleForm extends Component {
       tagList: this.article.cached_tag_list || '',
       description: '',
       canonicalUrl: this.article.canonical_url || '',
-      series:this.state.series || '',
+      series: this.state.series || '',
       allSeries: this.article.all_series || [],
       bodyMarkdown: this.article.body_markdown || '',
       published: this.article.published || false,
@@ -292,7 +292,7 @@ export default class ArticleForm extends Component {
       tagList: this.article.cached_tag_list || '',
       description: '',
       canonicalUrl: this.article.canonical_url || '',
-      series:this.state.series || '',
+      series: this.state.series || '',
       allSeries: this.article.all_series || [],
       bodyMarkdown: this.article.body_markdown || '',
       published: this.article.published || false,
@@ -381,14 +381,17 @@ export default class ArticleForm extends Component {
     );
     const orgArea =
       organizations && organizations.length > 0 ? (
-        <OrgSettings
-          organizations={organizations}
-          organizationId={organizationId}
-          onToggle={this.handleOrgIdChange}
-        />
-      ) : (
-        ''
-      );
+        <div className="articleform__orgsettings">
+          Publish under an organization:
+          <OrganizationPicker
+            name="article[organization_id]"
+            id="article_publish_under_org"
+            organizations={organizations}
+            organizationId={organizationId}
+            onToggle={this.handleOrgIdChange}
+          />
+        </div>
+      ) : null;
     const errorsArea = errors ? <Errors errorsList={errors} /> : '';
     let editorView = '';
     if (previewShowing) {
diff --git a/app/javascript/article-form/elements/orgSettings.jsx b/app/javascript/article-form/elements/orgSettings.jsx
deleted file mode 100644
index 51467ae48..000000000
--- a/app/javascript/article-form/elements/orgSettings.jsx
+++ /dev/null
@@ -1,51 +0,0 @@
-import { h } from 'preact';
-import PropTypes from 'prop-types';
-
-const orgOptions = (organizations, organizationId) => {
-  const orgs = organizations.map(organization => {
-    if (organizationId === organization.id) {
-      return (
-        <option value={organization.id} selected>
-          {organization.name}
-        </option>
-      );
-    }
-    return <option value={organization.id}>{organization.name}</option>;
-  });
-  const nullOrgOption =
-    organizationId === null ? (
-      <option value="" selected>
-        None
-      </option>
-    ) : (
-      <option value="">None</option>
-    );
-  orgs.unshift(nullOrgOption); // make first option as "None"
-  return orgs;
-};
-
-const OrgSettings = ({ organizations, organizationId, onToggle }) => (
-  <div className="articleform__orgsettings">
-    Publish under an organization:
-    <select
-      name="article[organization_id]"
-      id="article_publish_under_org"
-      onBlur={onToggle}
-    >
-      {orgOptions(organizations, organizationId)}
-    </select>
-  </div>
-);
-
-OrgSettings.propTypes = {
-  onToggle: PropTypes.func.isRequired,
-  organizationId: PropTypes.string.isRequired,
-  organizations: PropTypes.arrayOf(
-    PropTypes.shape({
-      id: PropTypes.string,
-      name: PropTypes.string,
-    }),
-  ).isRequired,
-};
-
-export default OrgSettings;
diff --git a/app/javascript/listings/elements/orgSettings.jsx b/app/javascript/listings/elements/orgSettings.jsx
deleted file mode 100644
index 5fd9e4c5a..000000000
--- a/app/javascript/listings/elements/orgSettings.jsx
+++ /dev/null
@@ -1,55 +0,0 @@
-// listings
-import { h } from 'preact';
-import PropTypes from 'prop-types';
-
-const orgOptions = (organizations, organizationId) => {
-  const orgs = organizations.map(organization => {
-    if (organizationId === organization.id) {
-      return (
-        <option value={organization.id} selected>
-          {organization.name}
-        </option>
-      );
-    }
-    return <option value={organization.id}>{organization.name}</option>;
-  });
-  const nullOrgOption =
-    organizationId === null ? (
-      <option value="" selected>
-        None
-      </option>
-    ) : (
-      <option value="">None</option>
-    );
-  orgs.unshift(nullOrgOption); // make first option as "None"
-  return orgs;
-};
-
-const OrgSettings = ({ organizations, organizationId, onToggle }) => (
-  <div className="field">
-    <label htmlFor="organizationId">Post under an organization:</label>
-    <select
-      name="classified_listing[organization_id]"
-      id="listing_organization_id"
-      onBlur={onToggle}
-    >
-      {orgOptions(organizations, organizationId)}
-    </select>
-    <p>
-      <em>Posting on behalf of org spends org credits.</em>
-    </p>
-  </div>
-);
-
-OrgSettings.propTypes = {
-  onToggle: PropTypes.func.isRequired,
-  organizationId: PropTypes.number.isRequired,
-  organizations: PropTypes.arrayOf(
-    PropTypes.shape({
-      id: PropTypes.number.isRequired,
-      name: PropTypes.string.isRequired,
-    }),
-  ).isRequired,
-};
-
-export default OrgSettings;
diff --git a/app/javascript/listings/listingForm.jsx b/app/javascript/listings/listingForm.jsx
index 891dfcdc1..f20c7d0f8 100644
--- a/app/javascript/listings/listingForm.jsx
+++ b/app/javascript/listings/listingForm.jsx
@@ -4,10 +4,10 @@ import linkState from 'linkstate';
 import Title from './elements/title';
 import BodyMarkdown from './elements/bodyMarkdown';
 import Categories from './elements/categories';
-import OrgSettings from './elements/orgSettings';
 import ContactViaConnect from './elements/contactViaConnect';
 import ExpireDate from './elements/expireDate';
 import Tags from '../shared/components/tags';
+import { OrganizationPicker } from '../organization/OrganizationPicker';
 
 export default class ListingForm extends Component {
   constructor(props) {
@@ -63,14 +63,20 @@ export default class ListingForm extends Component {
 
     const selectOrg =
       organizations && organizations.length > 0 ? (
-        <OrgSettings
-          organizations={organizations}
-          organizationId={organizationId}
-          onToggle={this.handleOrgIdChange}
-        />
-      ) : (
-        ''
-      );
+        <div className="field">
+          <label htmlFor="organizationId">Post under an organization:</label>
+          <OrganizationPicker
+            name="classified_listing[organization_id]"
+            id="listing_organization_id"
+            organizations={organizations}
+            organizationId={organizationId}
+            onToggle={this.handleOrgIdChange}
+          />
+          <p>
+            <em>Posting on behalf of org spends org credits.</em>
+          </p>
+        </div>
+      ) : null;
 
     if (id === null) {
       return (
diff --git a/app/javascript/organization/OrganizationPicker.jsx b/app/javascript/organization/OrganizationPicker.jsx
new file mode 100644
index 000000000..048f5dfca
--- /dev/null
+++ b/app/javascript/organization/OrganizationPicker.jsx
@@ -0,0 +1,46 @@
+import { h } from 'preact';
+import PropTypes from 'prop-types';
+import { organizationPropType } from '../src/components/common-prop-types';
+
+const orgOptions = (organizations, organizationId) => {
+  const orgs = organizations.map(organization => {
+    if (organizationId === organization.id) {
+      return (
+        <option value={organization.id} selected>
+          {organization.name}
+        </option>
+      );
+    }
+    return <option value={organization.id}>{organization.name}</option>;
+  });
+  const nullOrgOption =
+    organizationId === null ? (
+      <option value="" selected>
+        None
+      </option>
+    ) : (
+      <option value="">None</option>
+    );
+
+  return [nullOrgOption, ...orgs];
+};
+
+export const OrganizationPicker = ({
+  name,
+  id,
+  organizations,
+  organizationId,
+  onToggle,
+}) => (
+  <select name={name} id={id} onBlur={onToggle}>
+    {orgOptions(organizations, organizationId)}
+  </select>
+);
+
+OrganizationPicker.propTypes = {
+  name: PropTypes.string.isRequired,
+  id: PropTypes.string.isRequired,
+  onToggle: PropTypes.func.isRequired,
+  organizationId: PropTypes.number.isRequired,
+  organizations: PropTypes.arrayOf(organizationPropType).isRequired,
+};
diff --git a/app/javascript/organization/__tests__/OrganizationPicker.test.jsx b/app/javascript/organization/__tests__/OrganizationPicker.test.jsx
new file mode 100644
index 000000000..2f858fde0
--- /dev/null
+++ b/app/javascript/organization/__tests__/OrganizationPicker.test.jsx
@@ -0,0 +1,49 @@
+import { h } from 'preact';
+import render from 'preact-render-to-json';
+import { OrganizationPicker } from '../OrganizationPicker';
+
+const commonProps = {
+  id: 'someFormElementId',
+  name: 'someFormElementName',
+  onToggle: jest.fn(),
+};
+
+const organizations = [
+  { id: 1, name: 'Acme Org 1' },
+  { id: 2, name: 'Acme Org 2' },
+];
+
+describe('<OrganizationPicker />', () => {
+  it('renders with the given organization selected from the list of available organizations', () => {
+    const tree = render(
+      <OrganizationPicker
+        {...commonProps}
+        organizationId={1}
+        organizations={organizations}
+      />,
+    );
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('renders with no organization selected when the organization ID is not set', () => {
+    const tree = render(
+      <OrganizationPicker
+        {...commonProps}
+        organizationId={undefined}
+        organizations={organizations}
+      />,
+    );
+    expect(tree).toMatchSnapshot();
+  });
+
+  it('renders an organization list with only "None" as an option when no organizations are passed in.', () => {
+    const tree = render(
+      <OrganizationPicker
+        {...commonProps}
+        organizationId={undefined}
+        organizations={[]}
+      />,
+    );
+    expect(tree).toMatchSnapshot();
+  });
+});
diff --git a/app/javascript/organization/__tests__/__snapshots__/OrganizationPicker.test.jsx.snap b/app/javascript/organization/__tests__/__snapshots__/OrganizationPicker.test.jsx.snap
new file mode 100644
index 000000000..819083269
--- /dev/null
+++ b/app/javascript/organization/__tests__/__snapshots__/OrganizationPicker.test.jsx.snap
@@ -0,0 +1,64 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<OrganizationPicker /> renders an organization list with only "None" as an option when no organizations are passed in. 1`] = `
+<select
+  id="someFormElementId"
+  name="someFormElementName"
+  onBlur={[MockFunction]}
+>
+  <option
+    value=""
+  >
+    None
+  </option>
+</select>
+`;
+
+exports[`<OrganizationPicker /> renders with no organization selected when the organization ID is not set 1`] = `
+<select
+  id="someFormElementId"
+  name="someFormElementName"
+  onBlur={[MockFunction]}
+>
+  <option
+    value=""
+  >
+    None
+  </option>
+  <option
+    value={1}
+  >
+    Acme Org 1
+  </option>
+  <option
+    value={2}
+  >
+    Acme Org 2
+  </option>
+</select>
+`;
+
+exports[`<OrganizationPicker /> renders with the given organization selected from the list of available organizations 1`] = `
+<select
+  id="someFormElementId"
+  name="someFormElementName"
+  onBlur={[MockFunction]}
+>
+  <option
+    value=""
+  >
+    None
+  </option>
+  <option
+    selected={true}
+    value={1}
+  >
+    Acme Org 1
+  </option>
+  <option
+    value={2}
+  >
+    Acme Org 2
+  </option>
+</select>
+`;
diff --git a/app/javascript/src/components/common-prop-types/index.js b/app/javascript/src/components/common-prop-types/index.js
index 0cfaaa393..c142e2194 100644
--- a/app/javascript/src/components/common-prop-types/index.js
+++ b/app/javascript/src/components/common-prop-types/index.js
@@ -1,4 +1,5 @@
 import userPropTypes from './user-prop-types';
 import defaultChildrenPropTypes from './default-children-prop-types';
 
+export * from './organization-prop-type';
 export { userPropTypes, defaultChildrenPropTypes };
diff --git a/app/javascript/src/components/common-prop-types/organization-prop-type.js b/app/javascript/src/components/common-prop-types/organization-prop-type.js
new file mode 100644
index 000000000..84b7dab86
--- /dev/null
+++ b/app/javascript/src/components/common-prop-types/organization-prop-type.js
@@ -0,0 +1,6 @@
+import PropTypes from 'prop-types';
+
+export const organizationPropType = PropTypes.shape({
+  id: PropTypes.number.isRequired,
+  name: PropTypes.string.isRequired,
+});