Add newPassword page by NadezhdaShosheva · Pull Request #230 · fs/react-next-base

NadezhdaShosheva · 2022-01-10T08:00:51Z

Summary

A brief description of the pull request.

Resolves: #138

Screenshots in case of UI changes

Review notes

While reviewing pull-request (especially when it's your pull-request), please make sure that:

you named PR close to issue name and linked that issue
you understand what problem is solved by PR and how is it solved
new tests are in place, no redundant tests
required ENV variables added and described in .env.example and added to Heroku
associated Heroku review app works correctly with introduced changes

Regina-Gimazova · 2022-01-10T13:08:30Z

api/mutations/update/useUpdatePasswordMutation.ts

+};
+
+const useUpdatePasswordMutation = (
+  options?: MutationHookOptions<UpdatePasswordResponseData, UpdatePasswordRequestVariables>,


Should the options be optional for mutation?

In this case it is required

…ew-password-form

diana-safyanova · 2022-01-11T13:28:24Z

components/pages/newPassword/NewPasswordPage.jsx

+
+NewPasswordPage.getInitialProps = ({ res, accessTokenManager }) => {
+  if (accessTokenManager.accessToken) {
+    if (res) {


I don't know why, but in other files we use the following conditions if (!!req && !!res) {

Now for all pages we check only res, so I think we can leave it

diana-safyanova · 2022-01-11T13:30:42Z

components/pages/newPassword/NewPasswordPage.jsx

+  return {};
+};
+
+export default withApolloClient(WithAuth(NewPasswordPage));


Why don't you use WithAuthSecurity HOC?

I think we can leave as is, cause almost all pages do not use this HOC. I'll take a look at this in next prs

diana-safyanova · 2022-01-11T13:34:07Z

components/pages/newPassword/components/NewPasswordForm/NewPasswordForm.tsx

+
+import { FieldWrapper, FormContentWrapper, SubmitButtonWrapper } from './styled';
+
+const passwordRegularExp = /^(?=.*[0-9])(?=.*[A-Z])(?=.*[a-z])([0-9A-Za-z#$@&!?.*^{}<>;,)(~'"=_%+-]+)$/;


better create new file with all regexp

diana-safyanova · 2022-01-11T13:41:55Z

components/pages/newPassword/components/NewPasswordForm/NewPasswordForm.tsx

+    .required('This field is required'),
+});
+
+type ValuesFromFormik = {


rename ValuesFromFormik to FormValues. If we replace formik with another form, type of values should not change, as well as name of type

diana-safyanova · 2022-01-11T13:42:11Z

components/pages/newPassword/components/NewPasswordForm/NewPasswordForm.tsx

+
+const passwordRegularExp = /^(?=.*[0-9])(?=.*[A-Z])(?=.*[a-z])([0-9A-Za-z#$@&!?.*^{}<>;,)(~'"=_%+-]+)$/;
+
+const initialValues = {


diana-safyanova · 2022-01-11T13:48:59Z

graphql/mutations/updatePassword.graphql

@@ -0,0 +1,5 @@
+mutation UpdatePassword($input: UpdatePasswordInput!) {
+  updatePassword(input: $input) {
+    accessToken


refreshToken

diana-safyanova · 2022-01-11T13:53:35Z

graphql/mutations/updatePassword.graphql

@@ -0,0 +1,5 @@
+mutation UpdatePassword($input: UpdatePasswordInput!) {


Oyyy I didn't know what type for $input existed
Maybe better to rename all input type for mutation

I'm sorry, but I don't understand your point. I think we can leave as is for now

diana-safyanova · 2022-01-11T13:56:08Z

lib/apollo/hooks/actions/useUpdatePassword.ts

+    onCompleted,
+  });
+
+  const mutate = async ({ password, resetToken }: UpdatePasswordProps) => {


UpdatePasswordProps should be named UpdatePasswordVariables from 'api/types...'

diana-safyanova · 2022-01-11T13:56:51Z

lib/apollo/hooks/actions/useUpdatePassword.ts

+
+import useUpdatePasswordMutation from 'api/mutations/update/useUpdatePasswordMutation';
+
+export type UpdatePasswordProps = {


Remove this type

diana-safyanova · 2022-01-11T14:10:59Z

components/pages/newPassword/NewPasswordPage.jsx

+const NewPasswordPage = ({ query }) => {
+  return (
+    <NotifierProvider>
+      <DefaultTemplate testId="recovery-password-page">


it is not recovery-password-page

DmitriyFirsov · 2022-01-13T08:25:59Z

api/mutations/update/useUpdatePasswordMutation.ts

+
+import UpdatePasswordMutation from 'graphql/mutations/updatePassword.graphql';
+
+import type { UpdatePasswordVariables, UpdatePasswordData } from '../../types/user/updatePasswordApiType';


should be the absolute path

add newPassword page

e2ad94f

Regina-Gimazova reviewed Jan 10, 2022

View reviewed changes

nadezhdakharchuk and others added 3 commits January 11, 2022 11:44

apply review comment

c01f3a9

Merge branch 'master' of https://github.com/fs/react-next-base into n…

8d5a744

…ew-password-form

[TS] small fix

e9065e6

diana-safyanova suggested changes Jan 11, 2022

View reviewed changes

Regina-Gimazova and others added 3 commits June 8, 2022 13:43

Merge branch 'master' into new-password-form

82a9e79

fix some comments

e1d8d4c

fix some comments

247c1dc

DmitriyFirsov suggested changes Jun 10, 2022

View reviewed changes


		import { FieldWrapper, FormContentWrapper, SubmitButtonWrapper } from './styled';

		const passwordRegularExp = /^(?=.[0-9])(?=.[A-Z])(?=.[a-z])([0-9A-Za-z#$@&!?.^{}<>;,)(~'"=_%+-]+)$/;


		const passwordRegularExp = /^(?=.[0-9])(?=.[A-Z])(?=.[a-z])([0-9A-Za-z#$@&!?.^{}<>;,)(~'"=_%+-]+)$/;

		const initialValues = {

		@@ -0,0 +1,5 @@
		mutation UpdatePassword($input: UpdatePasswordInput!) {


		import useUpdatePasswordMutation from 'api/mutations/update/useUpdatePasswordMutation';

		export type UpdatePasswordProps = {


		import UpdatePasswordMutation from 'graphql/mutations/updatePassword.graphql';

		import type { UpdatePasswordVariables, UpdatePasswordData } from '../../types/user/updatePasswordApiType';

Conversation

NadezhdaShosheva commented Jan 10, 2022

Summary

A brief description of the pull request.

Resolves: #138

Screenshots in case of UI changes

Review notes

While reviewing pull-request (especially when it's your pull-request), please make sure that:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NadezhdaShosheva Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Regina-Gimazova Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Regina-Gimazova Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NadezhdaShosheva Jan 11, 2022 •

edited

Loading

Regina-Gimazova Jun 8, 2022 •

edited

Loading

Regina-Gimazova Jun 8, 2022 •

edited

Loading