Convert TaxCloud panel to React #3121

foladipo · 2017-10-17T19:15:48Z

Resolves #3093 .

This PR converts the TaxCloud settings panel from using AutoForm to using React for editing its settings.

Test Instructions

Login as admin.
Open the TaxCloud settings panel. Observe the current API Login ID and API Key values (if any).
Change the login ID or/and API key in the TaxCloud settings form and submit the form. Observe an alert that confirms that the new settings were saved successfully.
Refresh the page and open the TaxCloud panel again. Observe that the form now contains the login ID or API key you entered above.
Note: Instead of refreshing the page, you can use RoboMongo (or similar) to inspect the DB. Just search the Package collection for the package with name: taxes-taxcloud. Then cycle between editing the TaxCloud settings form and checking the settings.taxcloud field in the said package. You'll notice that that field always changes as appropriate.

- add some i18n text

brent-hoover

This is sort of an incomplete conversion. We should be converting this to the container/component pattern and eliminate all the Blaze we can.

brent-hoover · 2017-10-20T00:12:37Z

imports/plugins/included/taxes-taxcloud/client/components/taxCloudSettingsForm.js

+  );
+};
+
+TaxCloudSettingsForm.propTypes = {


Let's document propTypes like here:
https://github.com/reactioncommerce/reaction-jsdoc/blob/master/README.md#document-a-react-components

brent-hoover · 2017-10-20T00:14:23Z

imports/plugins/included/taxes-taxcloud/client/settings/taxcloud.js

+ * @since 1.5.1
+ * @return {Object} - returns the data found for the said Package.
+ */
+function getPackageData(pkgName) {


Let's use this function instead:

reaction/server/api/core/core.js

Line 434 in 8521e1d

getPackageSettings(name) {

So I think we want to pretty much eliminate all this code in the old template file and replace this with a container and just have a placeholder that renders the container.

foladipo · 2017-10-20T10:57:03Z

@zenweasel Roger that. Implementing those reviews right away.

brent-hoover · 2017-10-20T11:23:42Z

This is the container example I was talking about

reaction/imports/plugins/core/checkout/client/containers/completedOrderContainer.js

Line 1 in 7ead8c0

import { compose, withProps } from "recompose";

See how it relates to the component here:

reaction/imports/plugins/core/checkout/client/components/completedOrder.js

Line 3 in 57553e8

import { Components } from "@reactioncommerce/reaction-components";

foladipo · 2017-10-22T20:16:05Z

Roger that @zenweasel . That example is really helpful.

foladipo · 2017-10-22T20:16:58Z

Status update

This PR is currently WIP because I'm busy implementing the changes above.

foladipo · 2017-10-23T10:46:30Z

Status update

Two of the reviews above have been implemented. The one that remains is to create and use the container component discussed above.

foladipo · 2017-10-24T10:19:05Z

Status update

Review #3 has been implemented. Will soon remove the [WIP] tag, but only after some more testing/linting.

Suggested method is not the same as requirements

foladipo · 2017-10-31T23:42:21Z

Roger that @zenweasel . I'll work on adding tests for the packages/update method.

foladipo · 2017-11-01T17:15:32Z

@zenweasel After adding the specs for package/update, how do I run the tests in that file alone?

foladipo · 2017-11-01T23:40:33Z

@zenweasel I have pushed the tests for packages/update. Please review.

NB: For now, I used describe.only and it.only while writing these tests. This will allow us to focus on running, reviewing and improving these tests, instead of those of the entire project. But once the review process is done, I'll revert to using describe and it.

…ng type

foladipo · 2017-11-07T16:43:58Z

UPDATE

I added a spec for when package/update is called with arguments of the wrong type.

cc @zenweasel

machikoyasuda

The JSDoc comments look good to me!

mikemurray · 2017-11-08T17:02:20Z

imports/plugins/included/taxes-taxcloud/client/components/taxCloudSettingsForm.js

+    <div className="rui taxcloud-settings-form">
+      {!settings.taxcloud.apiLoginId &&
+        <div className="alert alert-info">
+          <span data-i18n="admin.taxSettings.taxcloudCredentials">Add API Login ID to enable</span>


Don't use data-i18n use the translation component <Components.Translation defaultValue="" i18nkey="" />

brent-hoover · 2017-11-08T22:45:04Z

@mikemurray Thanks for catching the data-i18n. In the interest of getting this merged I made the requested change. If that's correct can you review and approve?

brent-hoover · 2017-11-09T23:57:59Z

@mikemurray Can you double-check that my changes are good and we can approve?

brent-hoover · 2017-11-10T01:06:25Z

Thanks @mikemurray

…ud-panel-to-react-3093 Convert TaxCloud panel to React

foladipo added 12 commits October 12, 2017 20:31

create component for changing taxcloud settings

2a0e841

replace autoform with react component for updating taxcloud settings

25e8e71

improve the name of a certain template helper

3ac4445

add jsdocs

62b4e8a

create a Meteor method for updating the data for a certain Package

7e207c1

- enable the new TaxCloud settings component to update DB values

ad232c2

- add some i18n text

add jsdocs

f896862

remove obsolete autoform code

95022d8

add a missing return statement

e07e2d0

code linting

5771d1a

improve the handling errors from trying to update the TaxCloud package

f765915

improve error handling

6de1358

foladipo changed the title ~~[WIP] Convert TaxCloud panel to React~~ Convert TaxCloud panel to React Oct 19, 2017

foladipo requested a review from brent-hoover October 19, 2017 14:03

brent-hoover requested changes Oct 20, 2017

View reviewed changes

foladipo changed the title ~~Convert TaxCloud panel to React~~ [WIP] Convert TaxCloud panel to React Oct 22, 2017

foladipo added 2 commits October 22, 2017 21:53

add jsdocs for a component's proptypes and its source file's module

336a7f0

replace some functions with Reaction.getPackageSettings()

bbda8bf

foladipo added 3 commits October 23, 2017 20:37

create TaxCloudSettingsFormContainer

cf4e711

replace some code with the use of TaxCloudSettingsFormContainer

a75cdb6

add/update jsdocs

cb310f4

foladipo added 3 commits October 24, 2017 16:40

improve jsdoc

1cd18ee

add missing prop

10a12e3

correct i18n values

31de683

write tests for the package/update Meteor method

8b218fe

add tests for when package/update is called with arguments of the wro…

a8aa5c7

…ng type

Remove only from tests so all tests run as normal

ab24292

brent-hoover previously approved these changes Nov 8, 2017

View reviewed changes

machikoyasuda previously approved these changes Nov 8, 2017

View reviewed changes

mikemurray requested changes Nov 8, 2017

View reviewed changes

Remove data-i18n and replace with Components.Translation

bba0eb5

brent-hoover dismissed stale reviews from machikoyasuda and themself via bba0eb5 November 8, 2017 22:43

mikemurray approved these changes Nov 10, 2017

View reviewed changes

This was referenced Nov 20, 2017

Convert Search settings panel to React #3323

Closed

Convert SMS settings panel to React #3324

Closed

Convert Authorize.net Settings panel to React #3325

Closed

spencern changed the base branch from master to release-1.5.9 November 20, 2017 23:57

spencern merged commit 83b7b04 into release-1.5.9 Nov 20, 2017

spencern deleted the folusho-convert-taxcloud-panel-to-react-3093 branch November 20, 2017 23:58

spencern mentioned this pull request Nov 21, 2017

Release 1.5.9 #3333

Merged

Akarshit mentioned this pull request Nov 21, 2017

Convert taxes-avalara to React #3334

Closed

brent-hoover mentioned this pull request Nov 23, 2017

Convert Braintree settings panel to React #3344

Closed

brent-hoover mentioned this pull request Dec 20, 2017

Convert the TaxCloud panel to a React component #3093

Closed

Akarshit pushed a commit that referenced this pull request Jan 7, 2018

Merge pull request #3121 from reactioncommerce/folusho-convert-taxclo…

983fffa

…ud-panel-to-react-3093 Convert TaxCloud panel to React

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert TaxCloud panel to React #3121

Convert TaxCloud panel to React #3121

foladipo commented Oct 17, 2017 •

edited

Loading

brent-hoover left a comment

brent-hoover Oct 20, 2017

brent-hoover Oct 20, 2017

brent-hoover Oct 20, 2017

foladipo commented Oct 20, 2017

brent-hoover commented Oct 20, 2017

foladipo commented Oct 22, 2017

foladipo commented Oct 22, 2017

foladipo commented Oct 23, 2017

foladipo commented Oct 24, 2017

foladipo commented Oct 31, 2017

foladipo commented Nov 1, 2017

foladipo commented Nov 1, 2017

foladipo commented Nov 7, 2017

machikoyasuda left a comment

mikemurray Nov 8, 2017

brent-hoover commented Nov 8, 2017

brent-hoover commented Nov 9, 2017

brent-hoover commented Nov 10, 2017

Convert TaxCloud panel to React #3121

Convert TaxCloud panel to React #3121

Conversation

foladipo commented Oct 17, 2017 • edited Loading

Test Instructions

brent-hoover left a comment

Choose a reason for hiding this comment

brent-hoover Oct 20, 2017

Choose a reason for hiding this comment

brent-hoover Oct 20, 2017

Choose a reason for hiding this comment

brent-hoover Oct 20, 2017

Choose a reason for hiding this comment

foladipo commented Oct 20, 2017

brent-hoover commented Oct 20, 2017

foladipo commented Oct 22, 2017

foladipo commented Oct 22, 2017

Status update

foladipo commented Oct 23, 2017

Status update

foladipo commented Oct 24, 2017

Status update

foladipo commented Oct 31, 2017

foladipo commented Nov 1, 2017

foladipo commented Nov 1, 2017

foladipo commented Nov 7, 2017

UPDATE

machikoyasuda left a comment

Choose a reason for hiding this comment

mikemurray Nov 8, 2017

Choose a reason for hiding this comment

brent-hoover commented Nov 8, 2017

brent-hoover commented Nov 9, 2017

brent-hoover commented Nov 10, 2017

foladipo commented Oct 17, 2017 •

edited

Loading