Feature/account page #900

anmol-fzr · 2024-05-10T18:21:03Z

Issue

Issue Number: 865

Issue Link: #865

What changes did you make?

Added Account Page with UI + Functionality + Translations. Delete Account button is intentionally left disabled reason being API to delete account by user isn't ready

Why did you make the changes?

Changes are made to fix the issue

Did you run tests?

Added a test named user-profile-update that ran successfully

Removed unused variable,functions and hooks calls from LoginForm

Added Only UI part of the Account Page described in issue chaynHQ#865

Refactored Signout logic code across 3 files using custom hook useAuth

Added Functionality for profile data updates

Added Functionality to update email preferences in account settings

Added Caution modal for account deletion

…ountSettings

Disabled delete Account Button as API is not made to delete user on its own

vercel · 2024-05-10T18:21:06Z

@anmol-fzr is attempting to deploy a commit to the Chayn Team on Vercel.

A member of the Team first needs to authorize it.

Fixed Strings must use Single Quote Error by elint

eleanorreem · 2024-05-11T16:13:12Z

Thanks so much for this, loads of work 🎉

I know that this PR is WIP so apologies for all the comments. I had a look through the code in combination with the current functionality that we had in the backend. These are my comments:

The backend for updating an email isn't implemented so for now it is best just to have the Profile details form as per the designs (i.e. add the link in the issue to the getting in touch text and remove the button). If you would like to contribute this work at a later date, I can tag you in the issue when it comes up and you can simply implement the work you have already done. Let me know if that works for you!
The backend for deleting an account isn't ready yet and is going to be a bit complicated. For the sake of getting this merged soon, I would remove the Account Actions section for the PR and save it for the next issue that I will write up in the near future.
The email subscription form works well. I am happy with it. We will need to have some error / success feedback for the user. Please see the bloom-frontend/components/forms/CreateAccessCodeForm.tsx for some input on how you might implement that. For now the success can just be "Your email subscription preferences have been updated"
There are some console logs and commented out code that will need tidying up.
As you have included more functionality in this PR than in the original ticket, you would need to add more functionality to the cypress test. In the new cypress test, you will need to check the profile details and that updating your subscription preferences works.
The header h1 and description isn't quite aligned with the middle of the image. You might need to add a marginTop: auto and a marginBottom: auto the top and bottom of the div that contains the header and description. I hope that makes sense.
There are errors when I run yarn build and yarn lint so it would be great if you could sort the single quotes.
I don't know if you experience this but the page flashes on load which doesn't seem to happen to other pages.I think its because some code has been copied over from the disable service emails page that shouldn't be there. The whole use effect can go.
When you have made all the changes, it would be awesome if you could soft reset the branch and redo the commits, excluding the features we aren't going to include in this PR otherwise it might get confusing that we are adding features and then taking them away.

Sorry for the long list of comments! What you have done so far is great. Thank you for all your work. Please do keep the work that isn't going to be included in this PR and we can use it in the very near future.

guards/publicPageDataWrapper.tsx

eleanorreem · 2024-05-12T08:07:27Z

components/forms/LoginForm.tsx

    | React.ReactElement<any, string | React.JSXElementConstructor<any>>
  >();
  const [emailInput, setEmailInput] = useState<string>('');
  const [passwordInput, setPasswordInput] = useState<string>('');
-  const [getUser, { isLoading: getUserIsLoading }] = useGetUserMutation();
+  const [getUser] = useGetUserMutation();


Just wondering about these changes - I assume this code was unused?

eleanorreem · 2024-05-12T08:07:50Z

components/forms/LoginForm.tsx

@@ -91,7 +89,6 @@ const LoginForm = () => {
      })
      .catch((error) => {
        const errorCode = error.code;
-        const errorMessage = error.message;



Same here - assume this was unused code?

pages/account/settings.tsx

eleanorreem · 2024-05-12T08:25:31Z

components/account/EmailPref.tsx

+  const cPerms = useTypedSelector(state => state.user.contactPermission)
+  const sPerms = useTypedSelector(state => state.user.serviceEmailsPermission)
+
+  console.log({ cPerms, sPerms })


console.logs and commented out code

eleanorreem · 2024-05-12T08:26:46Z

pages/account/settings.tsx

+  const [updateUser, { isLoading: updateUserIsLoading }] = useUpdateUserMutation();
+
+  useEffect(() => {
+    if (userCreatedAt && userServiceEmailsPermission === true) {


I think I see where you have copied this from. This shouldn't be there, that functionality is only for the disable-service-email.tsx page.

eleanorreem · 2024-05-12T08:28:11Z

components/account/EmailPref.tsx

+    const payload = {
+      contactPermission, serviceEmailsPermission
+    }
+    updateUser(payload)


Add an await and ensure that we are handling the error/ success scenarios 😄

@anmol-fzr let me know if you have any questions about this change

eleanorreem · 2024-05-12T08:28:27Z

components/account/EmailPref.tsx

+                    name='sPerms'
+                    aria-label={t('checkbox.emailOnCourse')}
+                    defaultChecked={sPerms}
+                  // checked={emailPerms}


Please remove commented out code ⭐

@anmol-fzr let me know if you have any questions about this change

eleanorreem · 2024-05-12T08:28:55Z

cypress/integration/user-profile-update.cy.tsx

@@ -0,0 +1,32 @@
+describe('User should be able to update his profile', () => {
+  const publicEmail = Cypress.env("public_email")
+


See overall comment about what sort of functionality this should be testing. 😄

@anmol-fzr this cypress test doesn't address the scenarios that was specified in the ticket. Let me know if you have any questions!

pages/account/settings.tsx

eleanorreem · 2024-05-12T08:31:42Z

components/account/AccountActions.tsx

@@ -0,0 +1,49 @@
+import { Typography, CardContent, Card, Button } from '@mui/material'


For now, lets exclude this file from the PR. I can review this properly in the next iteration of this page. 🙏 Thanks so much for doing it though!

components/forms/ProfileDetailsForm.tsx

remove profile update feature and disabled inputs with pre-populated data as mention in Chat

update user-profile-update test to make sure that input is disabled

anmol-fzr · 2024-05-12T09:00:59Z

@eleanorreem I made the code changes suggested in the chat please take a look in to it.

pages/account/settings.tsx

eleanorreem · 2024-05-12T09:33:33Z

Thanks for your changes! They look great. There were a few more changes that were on PR. It would be great if they could be resolved. These are the ones outstanding:

removing account actions from the card
success/ error handling for saving profile information
The cypress test doesn't test whether the correct email is displaying, it just tests that the box is disabled. Ideally you would be checking against the email address and also checking that when you update your subscription preferences, it works.
Header alignment

removed account action from settings page

eleanorreem · 2024-05-17T07:24:41Z

@anmol-fzr, thanks for all your work so far! Just checking in here to make sure you have everything you need to finish off this ticket. Absolutely no pressure, it would just be great to get this contribution merged!

…lled

anmol-fzr · 2024-05-17T12:21:53Z

@eleanorreem , I made the changes to the code suggest in this review please check.

eleanorreem · 2024-05-17T13:17:12Z

components/forms/ProfileDetailsForm.tsx

+    return
+  }
+
+  verifyBeforeUpdateEmail(currUser, newEmail).then((res) => {


On email update is no longer in use so should probably be removed.

eleanorreem · 2024-05-17T13:18:27Z

Hi thank you for your quick reply and all the work so far! There were a few more things that I commented on your pull request - take a look through the pull request files. There are some unresolved comments that need to be addressed before I can merge.

anmol-fzr · 2024-05-19T19:05:43Z

@eleanorreem I made change told last time about removing account action card to header alignment. It would be great if you review the code for merge or we resolve anything that needs to be done for this PR

eleanorreem · 2024-05-20T13:01:19Z

Hiya @anmol-fzr, Thanks for your response 😄

There are these open comments on your PR. Let me know if you have any questions. I linked to the most of the comments here.

Can you delete this file
Can you add an await to ensure this function is async and ensure that there is a success and failure scenrio. I have signposted in the comment where to look for examples.
You've left commented out code which would be great to be removed.
I had a question about some changes
Please note the cypress test doesn’t cover the functionality of updating your subscription preferences, specified in the comments above.

Please check through the open comments and make sure they are all resolved.

Thanks in advance for everything !

Removed Account Actions Component will do that in some other PR

anmol-fzr · 2024-05-20T16:29:17Z

@eleanorreem i made suggested changes, please check

anmol-fzr and others added 15 commits May 8, 2024 22:53

Fix: Removed unused Variables

73467c7

Removed unused variable,functions and hooks calls from LoginForm

Add: UI for Account Page

e6209d0

Added Only UI part of the Account Page described in issue chaynHQ#865

Refactor: User Logout Code

6a3df00

Refactored Signout logic code across 3 files using custom hook useAuth

Add: API for profile updation

994a4d0

Added Functionality for profile data updates

Merge branch 'chaynHQ:develop' into feature/account-page

e5b42b1

Add: Email Prefs Update func

6fee191

Added Functionality to update email preferences in account settings

Add: UI for Account Deletion

ef70f98

Added Caution modal for account deletion

Remove unused imports and refactor code in ProfileDetailsForm and Acc…

b6818f6

…ountSettings

Add deleteUserMutation to API

1f715f5

Add account settings translations for multiple languages

915a759

Add account settings translations

7e123d8

Update delete account button label translations

a8ad55c

Add user profile update test case

68759d4

Fix: disabled delete account button

e62bd75

Disabled delete Account Button as API is not made to delete user on its own

Merge branch 'chaynHQ:develop' into feature/account-page

962651a

Fix: Linting Errors

92673f3

Fixed Strings must use Single Quote Error by elint

eleanorreem reviewed May 12, 2024

View reviewed changes

guards/publicPageDataWrapper.tsx Show resolved Hide resolved

eleanorreem reviewed May 12, 2024

View reviewed changes

pages/account/settings.tsx Outdated Show resolved Hide resolved

eleanorreem reviewed May 12, 2024

View reviewed changes

pages/account/settings.tsx Outdated Show resolved Hide resolved

eleanorreem reviewed May 12, 2024

View reviewed changes

components/forms/ProfileDetailsForm.tsx Show resolved Hide resolved

anmol-fzr added 4 commits May 12, 2024 14:08

Fix: Code Suggestions in Code Review

1aa202f

Removed: Unneccessay code, comments and log statements

6c6d761

Removed: profile update functionality

f1f83f9

remove profile update feature and disabled inputs with pre-populated data as mention in Chat

Tests: fix testing code to match with functionality

1fc3ef3

update user-profile-update test to make sure that input is disabled

eleanorreem reviewed May 12, 2024

View reviewed changes

pages/account/settings.tsx Outdated Show resolved Hide resolved

Removed: Account Actions

a17f6aa

removed account action from settings page

kyleecodes self-assigned this May 15, 2024

anmol-fzr added 2 commits May 17, 2024 17:49

Test: add update profile test to check for name and email to be prefi…

4cff5d1

…lled

Fix: Margin of EmailPref

079d538

eleanorreem reviewed May 17, 2024

View reviewed changes

This was referenced May 17, 2024

Refactor Session and Session_iba template #907

Open

Create maintenance page #413

Open

anmol-fzr added 6 commits May 20, 2024 18:40

Deleted: Account Actions Component

244a0ae

Removed Account Actions Component will do that in some other PR

Removed - Comments

b2cd7de

Add: Error Handling to profile update

2c2bb33

Moved: Profile details to seperate component

b3ae012

Addon to prev commit

04313cb

test: Add email pref update to update-user-test

839acf2

Merge branch 'develop' into feature/account-page

8d9ef14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/account page #900

Feature/account page #900

anmol-fzr commented May 10, 2024

vercel bot commented May 10, 2024

eleanorreem commented May 11, 2024 •

edited

eleanorreem May 12, 2024

eleanorreem May 12, 2024

eleanorreem May 12, 2024

eleanorreem May 12, 2024

eleanorreem May 12, 2024

eleanorreem May 17, 2024

eleanorreem May 12, 2024 •

edited

eleanorreem May 17, 2024

eleanorreem May 12, 2024 •

edited

eleanorreem May 17, 2024

eleanorreem May 12, 2024 •

edited

anmol-fzr commented May 12, 2024

eleanorreem commented May 12, 2024 •

edited

eleanorreem commented May 17, 2024

anmol-fzr commented May 17, 2024

eleanorreem May 17, 2024

eleanorreem commented May 17, 2024

anmol-fzr commented May 19, 2024

eleanorreem commented May 20, 2024

anmol-fzr commented May 20, 2024

		@@ -0,0 +1,32 @@
		describe('User should be able to update his profile', () => {
		const publicEmail = Cypress.env("public_email")

		@@ -0,0 +1,49 @@
		import { Typography, CardContent, Card, Button } from '@mui/material'

Feature/account page #900

Are you sure you want to change the base?

Feature/account page #900

Conversation

anmol-fzr commented May 10, 2024

Issue

Issue Number: 865

Issue Link: #865

What changes did you make?

Why did you make the changes?

Did you run tests?

vercel bot commented May 10, 2024

eleanorreem commented May 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eleanorreem May 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eleanorreem May 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eleanorreem May 12, 2024 • edited

Choose a reason for hiding this comment

anmol-fzr commented May 12, 2024

eleanorreem commented May 12, 2024 • edited

eleanorreem commented May 17, 2024

anmol-fzr commented May 17, 2024

Choose a reason for hiding this comment

eleanorreem commented May 17, 2024

anmol-fzr commented May 19, 2024

eleanorreem commented May 20, 2024

anmol-fzr commented May 20, 2024

eleanorreem commented May 11, 2024 •

edited

eleanorreem May 12, 2024 •

edited

eleanorreem May 12, 2024 •

edited

eleanorreem May 12, 2024 •

edited

eleanorreem commented May 12, 2024 •

edited