Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

O3-1465: Disable the subscribe to snapshot option when version is provided #13

Merged
merged 2 commits into from Aug 31, 2022
Merged

O3-1465: Disable the subscribe to snapshot option when version is provided #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2543d67
O3-1465: Disable subscribe to snapshot option when version is provided
Piumal1999 Aug 26, 2022
d222420
Update url parts count logic
Piumal1999 Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 24 additions & 1 deletion packages/esm-admin-openconceptlab-app/src/subscription/subscription.component.tsx
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ import { useTranslation } from 'react-i18next';
import { deleteSubscription, updateSubscription, useSubscription } from './subscription.resource';
import styles from './subscription.component.scss';
import { useSWRConfig } from 'swr';
import { isVersionDefinedInUrl } from '../utils';

const Subscription: React.FC = () => {
const { t } = useTranslation();
Expand All @@ -26,6 +27,7 @@ const Subscription: React.FC = () => {
const [token, setToken] = useState('');
const [isSubscribedToSnapshot, setIsSubscribedToSnapshot] = useState(false);
const [validationType, setValidationType] = useState<'NONE' | 'FULL'>('FULL');
const [isSnapshotOptionDisabled, setIsSnapshotOptionDisabled] = useState(false);

const { data: subscription, isLoading, isError } = useSubscription();

Expand All @@ -35,11 +37,18 @@ const Subscription: React.FC = () => {
setToken(subscription?.token || '');
setIsSubscribedToSnapshot(subscription?.subscribedToSnapshot || false);
setValidationType(subscription?.validationType || 'FULL');
setIsSnapshotOptionDisabled(subscription ? isVersionDefinedInUrl(subscription.url) : false);
}
}, [isLoading, isError, subscription]);

const handleChangeSubscriptionUrl = useCallback((event: React.ChangeEvent<HTMLInputElement>) => {
setSubscriptionUrl(event.target.value);
if (isVersionDefinedInUrl(event.target.value)) {
setIsSnapshotOptionDisabled(true);
setIsSubscribedToSnapshot(false);
} else {
setIsSnapshotOptionDisabled(false);
}
}, []);

const handleChangeToken = useCallback((event: React.ChangeEvent<HTMLInputElement>) => {
Expand All @@ -59,6 +68,17 @@ const Subscription: React.FC = () => {
evt.preventDefault();
evt.stopPropagation();

if (isSnapshotOptionDisabled && isSubscribedToSnapshot) {
showNotification({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we could do something with a modal dialog box instead? I.e., "You can't subscribe to a snapshot release if you've subscribed to a specific collection version. Do you want to remain subscribed to a specific version or switch to a snapshot?"

Also, it would be best to change "SNAPSHOT" -> "HEAD". I know the old UI was using SNAPSHOT, but OCL uses HEAD for the same concept, so it would probably be more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we could do something with a modal dialog box instead? I.e., "You can't subscribe to a snapshot release if you've subscribed to a specific collection version. Do you want to remain subscribed to a specific version or switch to a snapshot?"

@ibacher Actually, this error is already prevented by disabling the snapshot checkbox when a version is given with the URL. So I thought a notification would be enough. If we are displaying a modal, do I have to use the showModal from @openmrs/esm-framework?

Also, it would be best to change "SNAPSHOT" -> "HEAD". I know the old UI was using SNAPSHOT, but OCL uses HEAD for the same concept, so it would probably be more understandable.

Shall I rename all the occurrences of SNAPSHOT to HEAD?

kind: 'error',
description: t(
'snapshotDisabledError',
"You cannot subscribe to a SNAPSHOT if you've provided the collection version",
),
});
return;
}

const abortController = new AbortController();

const updatedSubscription = {
Expand Down Expand Up @@ -92,14 +112,15 @@ const Subscription: React.FC = () => {

return () => abortController.abort();
},
[subscriptionUrl, token, validationType, isSubscribedToSnapshot, t, subscription, mutate],
[subscriptionUrl, token, validationType, isSubscribedToSnapshot, isSnapshotOptionDisabled, subscription, t, mutate],
);

const handleCancel = useCallback(() => {
setSubscriptionUrl(subscription?.url || '');
setToken(subscription?.token || '');
setIsSubscribedToSnapshot(subscription?.subscribedToSnapshot || false);
setValidationType(subscription?.validationType || 'FULL');
setIsSnapshotOptionDisabled(subscription ? isVersionDefinedInUrl(subscription.url) : false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're repeating this code multiple times in different places, it seems like maybe this should just be a useEffect() hook?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is already included in the useEffect() hook in lines 34-40. Is there a way to trigger it when calling this handleCancel method?


showNotification({
kind: 'info',
Expand All @@ -121,6 +142,7 @@ const Subscription: React.FC = () => {
setToken('');
setIsSubscribedToSnapshot(false);
setValidationType('FULL');
setIsSnapshotOptionDisabled(false);
showNotification({
kind: 'success',
description: t('subscriptionDeleted', 'Successfully unsubscribed'),
Expand Down Expand Up @@ -205,6 +227,7 @@ const Subscription: React.FC = () => {
onChange={handleChangeSubscriptionType}
labelText={t('subscribeToSnapshotText', 'Subscribe to SNAPSHOT versions (not recommended)')}
id="isSubscribedToSnapshot"
disabled={isSnapshotOptionDisabled}
/>
<Checkbox
checked={validationType === 'NONE'}
Expand Down
27 changes: 27 additions & 0 deletions packages/esm-admin-openconceptlab-app/src/utils.ts
View file
Open in desktop
@@ -0,0 +1,27 @@
const NUMBER_OF_SLASHES_AFTER_BASE_URL = 5;

/*
* This checks if collection version has been passed to subscription url by checking number of forward slashes after base url
* If the number is 5, such as with https://api.openconceptlab.org/users/username/collections/collectionname/v1.0
* that means collection version was passed and isVersionDefinedInUrl() will return true
* Also returns false if the string is not a valid URL
*/
export const isVersionDefinedInUrl = (subscriptionUrl: string) => {
if (subscriptionUrl.endsWith('/')) {
subscriptionUrl = subscriptionUrl.substring(0, subscriptionUrl.lastIndexOf('/'));
}

let url;
try {
url = new URL(subscriptionUrl);
} catch (e) {
return false;
}

let count = url.pathname.match(/\//g)?.length ?? 0;
if (count == NUMBER_OF_SLASHES_AFTER_BASE_URL) {
return true;
} else {
return false;
}
};
1 change: 1 addition & 0 deletions packages/esm-admin-openconceptlab-app/translations/en.json
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@
"previousImportsFetchError": "Error occured while fetching the imports",
"result": "Result:",
"setupSubscription": "Setup Subscription",
"snapshotDisabledError": "You cannot subscribe to a SNAPSHOT if you've provided the collection version",
"startedOn": "Started on",
"status": "Status",
"subscribeButton": "Save changes",
Expand Down