CIF-1400 - MyAccount - Account Information Page #409

dplaton · 2020-09-28T08:45:42Z

Description

This PR adds the implementation of the AccountDetails components along with a few helper components. As a first, I included the GraphQL queries in the component's folders instead of the ./queries folder since that folder already grew too large.

I added a Dialog component which renders the edit form in a modal dialog. This component was directly taken from Venia.
I added a Password field component to render the password field which features a "toggle" that shows the password in clear case.

PR in Venia: adobe/aem-cif-guides-venia#63

Related Issue

CIF-1400

Motivation and Context

As a shopper, I want to update my personal data such as name, e-mail, and password.

How Has This Been Tested?

Prerequisites: fresh instance of AEM (on-prem or cloud)

1/ Install Venia with the development version of AEM Core CIF React Components
2/ Login with an existing user
3/ Click the "Account information" link.
4/ Check that the Account Details page opens correctly and retrieves the correct data
5/ Click the "Edit" button
6/ Check that the modal dialog opens and it's pre-filled with the correct data.
7/ Change any of the data (either name or e-mail) and click "Save".
8/ Check that the data is refreshed.
9/ Click the Edit button again
10/ Change the password - check that you get the validation error when you try to use the same password
11/ Click "Save"
12/ Sign out and sign in again with the new password - it should work.

Screenshots (if appropriate):

## Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes and the overall coverage did not decrease.
All unit tests pass on CircleCi.
I ran all tests locally and they pass.

Create the Account Details component in AEM

Update unit tests for the Account Details component

* add a form validator for the password field (along with unit tests) * switch to a named export for the Portal component * add unit tests

… the source file instead of from `index.js`

codecov · 2020-09-28T09:21:05Z

Codecov Report

Merging #409 into master will increase coverage by 0.17%.
The diff coverage is 88.97%.

@@             Coverage Diff              @@
##             master     #409      +/-   ##
============================================
+ Coverage     86.11%   86.28%   +0.17%     
  Complexity      950      950              
============================================
  Files           188      198      +10     
  Lines          4918     5053     +135     
  Branches        697      729      +32     
============================================
+ Hits           4235     4360     +125     
- Misses          541      551      +10     
  Partials        142      142

Flag	Coverage Δ	Complexity Δ
#integration	`67.29% <ø> (ø)`	`691.00 <ø> (ø)`
#jest	`82.32% <88.97%> (+0.77%)`	`0.00 <0.00> (ø)`
#karma	`94.24% <ø> (ø)`	`0.00 <ø> (ø)`
#unittests	`86.29% <ø> (ø)`	`919.00 <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
react-components/src/components/Button/button.js	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
react-components/src/context/ConfigContext.js	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...t-components/src/components/FormError/formError.js	`23.07% <23.07%> (ø)`	`0.00 <0.00> (?)`
...src/components/AccountContainer/accountDropdown.js	`89.28% <50.00%> (-6.72%)`	`0.00 <0.00> (ø)`
...-components/src/components/Password/usePassword.js	`80.00% <80.00%> (ø)`	`0.00 <0.00> (?)`
...src/components/AccountDetails/useAccountDetails.js	`94.59% <94.59%> (ø)`	`0.00 <0.00> (?)`
...ts/src/components/AccountDetails/accountDetails.js	`100.00% <100.00%> (ø)`	`0.00 <0.00> (?)`
...mponents/src/components/AccountDetails/editForm.js	`100.00% <100.00%> (ø)`	`0.00 <0.00> (?)`
...ponents/src/components/AccountDetails/editModal.js	`100.00% <100.00%> (ø)`	`0.00 <0.00> (?)`
react-components/src/components/Dialog/dialog.js	`100.00% <100.00%> (ø)`	`0.00 <0.00> (?)`
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d8e81d...98161f3. Read the comment docs.

herzog31 · 2020-09-28T10:52:59Z

I think if you move the queries out of the ./queries folder, it makes things like adobe/commerce-cif-graphql-integration-reference#30 much harder. So for now I would prefer to keep it in one place. @cjelger wdyt?

* Configure Jest to ignore all the graphql files in the project, regardless of location. * Fixed bug which was causing the update mutation to be run regardless if the data was changed or not.

cjelger · 2020-09-28T11:09:15Z

@herzog31 Definitely, and we also validate all queries with https://github.com/adobe/aem-core-cif-components/blob/master/react-components/src/queries/__test__/validateQueries.test.js so let's keep them all in that same folder.

* move the queries in the `queries` folder * fix a warning in an unit test

dplaton · 2020-09-28T11:31:34Z

@herzog31 @cjelger It's done, I moved the queries to the queries folder

…ponents into features/CIF-1400

herzog31 · 2020-09-29T08:00:30Z

react-components/src/components/AccountDetails/useAccountDetails.js

+import { useQuery, useMutation } from '@apollo/react-hooks';
+import { useUserContext } from '../../context/UserContext';
+
+const useAccountDetails = props => {


Since this hook stores state, I think it would be appropriate to use a React context here. So you don't need to pass all the exported logic and state here via props to child components like EditModal and EditForm.

There are only a few components under this tree, I don't see any reason to use a React Context just to manage the state of a modal. In general, I don't see any problem with passing state down to child components for trivial matters. I see React Contexts for cross-cutting concerns, like make some "context data" available to components that are not part of the same sub-tree.

herzog31 · 2020-09-29T08:13:04Z

react-components/src/utils/hooks/useScrollIntoView.js

+
+const OPTIONS_DEFAULTS = { behavior: 'smooth', block: 'center' };
+
+/**


Shouldn't we add a note or copyright header here, since this is taken from PWA studio?

The original component doesn't have a copyright header - https://github.com/magento/pwa-studio/blob/develop/packages/peregrine/lib/hooks/useScrollIntoView.js
I'll add a link to the original code

I think that's a problem actually. IIUC Magento uses OSL 3.0 which requires to publish any used code under OSL 3.0 as well which is incompatible with our Apache 2.0 license. I think we need to double check that.

Yes, they use OSL 3.0 which is not compatible with our license IIUC.
I'll revert the change and use a different method just to be safe.

…or it

…ponents into features/CIF-1400

* The password field is not cleared after the form submit * If a form error is thrown the error field is not cleared when the form is "Cancelled"

…o features/CIF-1400

Daniel Platon added 11 commits September 16, 2020 18:21

CIF-1400 - MyAccount - Account Information Page

21e8d06

Create the Account Details component in AEM

Create the AccountDetails React component and show read-only data.

220a653

CIF-1400 - MyAccount - Account Information Page

4ecf8f2

Update unit tests for the Account Details component

Create the edit dialog for the account details

bed971f

Add the "LinkButton" component that renders a button like a "link"

3a150e5

Add a "Password" field component

a99e9a5

Add a "FormError" component to display the errors in a form

68c0f9f

Add the capability to update the account information

90477e7

Add the "Change password" feature, update unit tests

c0b3c22

Account details:

12e5afc

* add a form validator for the password field (along with unit tests) * switch to a named export for the Portal component * add unit tests

Merge branch 'master' into features/CIF-1400

262730d

dplaton added the enhancement New feature or request label Sep 28, 2020

dplaton requested review from mhaack, herzog31, LSantha, laurentiumagureanu, cjelger and alkim91 September 28, 2020 08:45

dplaton mentioned this pull request Sep 28, 2020

CIF-1400 - MyAccount - Account Information Page adobe/aem-cif-guides-venia#63

Closed

10 tasks

alkim91 approved these changes Sep 28, 2020

View reviewed changes

Use a named export for the Account Details component

9728e6a

dplaton mentioned this pull request Sep 28, 2020

CIF-1400 - MyAccount - Account Information Page adobe/aem-cif-guides-venia#64

Merged

10 tasks

Daniel Platon added 2 commits September 28, 2020 11:57

Update the unit test of AccountDetails to import the component from…

19b66bc

… the source file instead of from `index.js`

Fix the wrong export

d2611b1

Account details:

62ffe85

* Configure Jest to ignore all the graphql files in the project, regardless of location. * Fixed bug which was causing the update mutation to be run regardless if the data was changed or not.

Account details:

647edb5

* move the queries in the `queries` folder * fix a warning in an unit test

dplaton and others added 4 commits September 28, 2020 14:31

Merge branch 'master' into features/CIF-1400

3278ad0

Add a unit test for the new deriveErrorMessage utility

e3e38d4

Merge branch 'features/CIF-1400' of github.com:adobe/aem-core-cif-com…

3cc146c

…ponents into features/CIF-1400

Merge branch 'master' into features/CIF-1400

6cf0a9f

herzog31 reviewed Sep 29, 2020

View reviewed changes

Daniel Platon added 2 commits September 29, 2020 15:12

Remove the useScrollIntoView hook since we don't have the license f…

eb5cfb1

…or it

Merge branch 'features/CIF-1400' of github.com:adobe/aem-core-cif-com…

f73e4a7

…ponents into features/CIF-1400

herzog31 approved these changes Sep 29, 2020

View reviewed changes

dplaton added the To Verify label Sep 30, 2020

Daniel Platon and others added 5 commits September 30, 2020 17:44

Update the translation files

079aef6

Fix bugs found in verification:

66cfd57

* The password field is not cleared after the form submit * If a form error is thrown the error field is not cleared when the form is "Cancelled"

Add README for the AEM component

3f97f0e

Fix the unit test for editForm

d593cf1

Merge branch 'master' of github.com:adobe/aem-core-cif-components int…

98161f3

…o features/CIF-1400

herzog31 added verified and removed To Verify labels Oct 1, 2020

herzog31 merged commit 6c566ae into master Oct 1, 2020

herzog31 deleted the features/CIF-1400 branch October 1, 2020 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CIF-1400 - MyAccount - Account Information Page #409

CIF-1400 - MyAccount - Account Information Page #409

dplaton commented Sep 28, 2020 •

edited

Loading

codecov bot commented Sep 28, 2020 •

edited

Loading

herzog31 commented Sep 28, 2020

cjelger commented Sep 28, 2020

dplaton commented Sep 28, 2020

herzog31 Sep 29, 2020

dplaton Sep 29, 2020

herzog31 Sep 29, 2020

dplaton Sep 29, 2020

herzog31 Sep 29, 2020

dplaton Sep 29, 2020 •

edited

Loading


		const OPTIONS_DEFAULTS = { behavior: 'smooth', block: 'center' };

		/**

CIF-1400 - MyAccount - Account Information Page #409

CIF-1400 - MyAccount - Account Information Page #409

Conversation

dplaton commented Sep 28, 2020 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

codecov bot commented Sep 28, 2020 • edited Loading

Codecov Report

herzog31 commented Sep 28, 2020

cjelger commented Sep 28, 2020

dplaton commented Sep 28, 2020

herzog31 Sep 29, 2020

Choose a reason for hiding this comment

dplaton Sep 29, 2020

Choose a reason for hiding this comment

herzog31 Sep 29, 2020

Choose a reason for hiding this comment

dplaton Sep 29, 2020

Choose a reason for hiding this comment

herzog31 Sep 29, 2020

Choose a reason for hiding this comment

dplaton Sep 29, 2020 • edited Loading

Choose a reason for hiding this comment

dplaton commented Sep 28, 2020 •

edited

Loading

codecov bot commented Sep 28, 2020 •

edited

Loading

dplaton Sep 29, 2020 •

edited

Loading