Skip to content

Commit

Permalink
Skip confirmation step on email subscription (#1646)
Browse files Browse the repository at this point in the history
* Email subscription params in request body

* Email subscription params in request body

fix tests

* Skip confirm step on email sub

When user logged in with the same email he tries to subscribe

* Skip confirm step on email sub

set autoConfirm param to make it work

* Update size-limit

* Handle 409: already subscribed

* refactor: prevStep to justSubscribed

prevStep is not used anywhere else and because of it influences output text (haveSubscribed), have changed it to more intuitive justSubscribed variable

* Test case for http error 409
  • Loading branch information
Jaskon authored Jun 29, 2023
1 parent 64188e5 commit 497f3ce
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 13 deletions.
16 changes: 13 additions & 3 deletions frontend/apps/remark42/app/common/api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import { siteId, url } from './settings';
import { BASE_URL, API_BASE } from './constants';
import { Config, Comment, Tree, User, BlockedUser, Sorting, BlockTTL, Image } from './types';
import {
Config,
Comment,
Tree,
User,
BlockedUser,
Sorting,
BlockTTL,
Image,
EmailSubVerificationStatus,
} from './types';
import { apiFetcher, adminFetcher } from './fetcher';

/* API methods */
Expand Down Expand Up @@ -72,8 +82,8 @@ export const uploadImage = (image: File): Promise<Image> => {
* Start process of email subscription to updates
* @param emailAddress email for subscription
*/
export const emailVerificationForSubscribe = (emailAddress: string) =>
apiFetcher.post('/email/subscribe', {}, { address: emailAddress });
export const emailVerificationForSubscribe = (emailAddress: string): Promise<EmailSubVerificationStatus> =>
apiFetcher.post('/email/subscribe', {}, { address: emailAddress, autoConfirm: true });

/**
* Confirmation of email subscription to updates
Expand Down
5 changes: 5 additions & 0 deletions frontend/apps/remark42/app/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,8 @@ export interface ApiError {
/** in-depth explanation */
error: string;
}

export interface EmailSubVerificationStatus {
updated: boolean;
address: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { persistEmail } from 'components/auth/auth.utils';
import enMessages from 'locales/en.json';

import { SubscribeByEmail, SubscribeByEmailForm } from '.';
import { RequestError } from '../../../utils/errorUtils';

const emailVerificationForSubscribeMock = emailVerificationForSubscribe as unknown as jest.Mock<
ReturnType<typeof emailVerificationForSubscribe>
Expand All @@ -29,6 +30,8 @@ const unsubscribeFromEmailUpdatesMock = unsubscribeFromEmailUpdates as unknown a
ReturnType<typeof unsubscribeFromEmailUpdates>
>;

emailVerificationForSubscribeMock.mockImplementation((email) => Promise.resolve({ address: email, updated: false }));

const initialStore = {
user,
theme: 'light',
Expand Down Expand Up @@ -104,7 +107,7 @@ describe('<SubscribeByEmailForm/>', () => {
expect(wrapper.text().startsWith('You are subscribed on updates by email')).toBe(true);
});

it('should pass throw subscribe process', async () => {
it('should pass through subscribe process', async () => {
const wrapper = createWrapper();

const input = wrapper.find('input');
Expand Down Expand Up @@ -138,6 +141,47 @@ describe('<SubscribeByEmailForm/>', () => {
expect(wrapper.find(Button).text()).toEqual('Unsubscribe');
});

it('should handle http error 409: already subscribed', async () => {
emailVerificationForSubscribeMock.mockImplementationOnce(() => Promise.reject(new RequestError('', 409)));

const wrapper = createWrapper();

const input = wrapper.find('input');
const form = wrapper.find('form');

input.getDOMNode<HTMLInputElement>().value = 'some@email.com';
input.simulate('input');
form.simulate('submit');

await sleep();
wrapper.update();

expect(wrapper.text().startsWith('You are subscribed on updates by email')).toBe(true);
});

it('should pass through subscribe process without confirmation', async () => {
emailVerificationForSubscribeMock.mockImplementationOnce((email) =>
Promise.resolve({ address: email, updated: true })
);

const wrapper = createWrapper();

const input = wrapper.find('input');
const form = wrapper.find('form');

input.getDOMNode<HTMLInputElement>().value = 'some@email.com';
input.simulate('input');
form.simulate('submit');

expect(emailVerificationForSubscribeMock).toHaveBeenCalledWith('some@email.com');

await sleep();
wrapper.update();

expect(wrapper.text().startsWith('You have been subscribed on updates by email')).toBe(true);
expect(wrapper.find(Button).text()).toEqual('Unsubscribe');
});

it('should fill in email from local storage', async () => {
const expected = 'someone@email.com';
persistEmail(expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { User } from 'common/types';
import { StoreState } from 'store';
import { setUserSubscribed } from 'store/user/actions';
import { sleep } from 'utils/sleep';
import { extractErrorMessageFromResponse } from 'utils/errorUtils';
import { extractErrorMessageFromResponse, RequestError } from 'utils/errorUtils';
import { useTheme } from 'hooks/useTheme';
import { getHandleClickProps } from 'common/accessibility';
import { emailVerificationForSubscribe, emailConfirmationForSubscribe, unsubscribeFromEmailUpdates } from 'common/api';
Expand Down Expand Up @@ -120,7 +120,7 @@ export const SubscribeByEmailForm: FunctionComponent = () => {
const subscribed = useSelector<StoreState, boolean>(({ user }) =>
user === null ? false : Boolean(user.email_subscription)
);
const previousStep = useRef<Step | null>(null);
const justSubscribed = useRef<Boolean>(false);

const [step, setStep] = useState(subscribed ? Step.Subscribed : Step.Email);

Expand All @@ -136,16 +136,31 @@ export const SubscribeByEmailForm: FunctionComponent = () => {
setError(null);

try {
let emailVerificationResponse;

switch (step) {
case Step.Email:
await emailVerificationForSubscribe(emailAddress);
try {
emailVerificationResponse = await emailVerificationForSubscribe(emailAddress);
justSubscribed.current = true;
} catch (e) {
if ((e as RequestError).code !== 409) {
throw e;
}
emailVerificationResponse = { address: emailAddress, updated: true };
}
if (emailVerificationResponse.updated) {
dispatch(setUserSubscribed(true));
setStep(Step.Subscribed);
break;
}
setToken('');
setStep(Step.Token);
break;
case Step.Token:
await emailConfirmationForSubscribe(currentToken);
dispatch(setUserSubscribed(true));
previousStep.current = Step.Token;
justSubscribed.current = true;
setStep(Step.Subscribed);
break;
default:
Expand Down Expand Up @@ -210,7 +225,6 @@ export const SubscribeByEmailForm: FunctionComponent = () => {
try {
await unsubscribeFromEmailUpdates();
dispatch(setUserSubscribed(false));
previousStep.current = Step.Subscribed;
setStep(Step.Unsubscribed);
} catch (e) {
// @ts-ignore
Expand All @@ -229,10 +243,9 @@ export const SubscribeByEmailForm: FunctionComponent = () => {
}

if (step === Step.Subscribed) {
const text =
previousStep.current === Step.Token
? intl.formatMessage(messages.haveSubscribed)
: intl.formatMessage(messages.subscribed);
const text = justSubscribed.current
? intl.formatMessage(messages.haveSubscribed)
: intl.formatMessage(messages.subscribed);

return (
<div className={b('comment-form__subscribe-by-email', { mods: { subscribed: true } })}>
Expand Down
4 changes: 4 additions & 0 deletions frontend/apps/remark42/app/utils/errorUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ export const errorMessages = defineMessages<string | number>({
id: 'errors.forbidden',
defaultMessage: 'Forbidden.',
},
409: {
id: 'errors.conflict',
defaultMessage: 'Conflict.',
},
429: {
id: 'errors.to-many-request',
defaultMessage: 'You have reached maximum request limit.',
Expand Down

0 comments on commit 497f3ce

Please sign in to comment.