Skip to content

Commit

Permalink
Shorten timetable share URL (#3367)
Browse files Browse the repository at this point in the history
  • Loading branch information
winphong authored Nov 20, 2024
1 parent c7638e1 commit e7452a7
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 91 deletions.
7 changes: 0 additions & 7 deletions website/src/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
/* eslint-disable import/prefer-default-export */

/**
* See:
* - https://github.com/nusmodifications/nusmods/issues/2057
* - https://github.com/nusmodifications/nusmods/pull/3212
*/
export const enableShortUrl = false;

/** Enable Course Planning Exercise */
export const enableCPEx = false;

Expand Down
13 changes: 6 additions & 7 deletions website/src/views/timetable/ShareTimetable.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import "~styles/utils/modules-entry.scss";
@import '~styles/utils/modules-entry.scss';

.header {
text-align: center;
Expand All @@ -10,7 +10,7 @@
}

h3 {
margin-bottom: 0.4rem;
margin: 0.4rem 0;
font-weight: $font-weight-bold;
font-size: 1.4rem;
}
Expand All @@ -27,11 +27,6 @@
margin-bottom: 2rem;
}

.copyIcon {
// Cannot use .btn-svg because display: inline-flex messes up button groups
margin-bottom: -0.2rem;
}

.copyStatus {
position: absolute;
right: 0;
Expand All @@ -53,6 +48,10 @@
text-align: center;
}

.buttonContainer {
width: 58px;
}

.qrCode {
max-width: 12rem;
padding: 0.2rem;
Expand Down
106 changes: 57 additions & 49 deletions website/src/views/timetable/ShareTimetable.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import axios, { AxiosResponse } from 'axios';
import { shallow, ShallowWrapper } from 'enzyme';
import { Maximize2, Minimize2 } from 'react-feather';

import { enableShortUrl } from 'featureFlags';
import { waitFor } from 'test-utils/async';
import Modal from 'views/components/Modal';
import LoadingSpinner from 'views/components/LoadingSpinner';
Expand Down Expand Up @@ -33,7 +33,7 @@ describe('ShareTimetable', () => {
},
};

const openModal = (wrapper: ShallowWrapper) => wrapper.find('button').simulate('click');
const openModal = (wrapper: ShallowWrapper) => wrapper.find('button').first().simulate('click');
const closeModal = (wrapper: ShallowWrapper) =>
wrapper.find(Modal).first().props().onRequestClose!({} as any);

Expand All @@ -54,64 +54,55 @@ describe('ShareTimetable', () => {

openModal(wrapper);
expect(wrapper.find(Modal).exists()).toBe(true);
if (enableShortUrl) {
expect(mockAxios.get).toHaveBeenCalledTimes(1);
} else {
expect(mockAxios.get).toHaveBeenCalledTimes(0);
}
expect(mockAxios.get).toHaveBeenCalledTimes(1);
});

if (enableShortUrl) {
test('should cache short URL from the API', () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);

// Open, close and open the modal again
openModal(wrapper);
closeModal(wrapper);
openModal(wrapper);

// The second open should not cause a second call
expect(mockAxios.get).toHaveBeenCalledTimes(1);
closeModal(wrapper);

// Changing the timetable should cause opening the modal to trigger another API call
wrapper.setProps({ timetable: { CS3216: { Lecture: '1' } } });
expect(mockAxios.get).toHaveBeenCalledTimes(1);
openModal(wrapper);
expect(mockAxios.get).toHaveBeenCalledTimes(2);
closeModal(wrapper);

// Changing the semester should also trigger another API call
wrapper.setProps({ semester: 2 });
expect(mockAxios.get).toHaveBeenCalledTimes(2);
openModal(wrapper);
expect(mockAxios.get).toHaveBeenCalledTimes(3);
});
test('should cache short URL from the API', () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);

test('should show spinner when loading', () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);
// Open, close and open the modal again
openModal(wrapper);
closeModal(wrapper);
openModal(wrapper);

openModal(wrapper);
expect(wrapper.find(LoadingSpinner).exists()).toBe(true);
});
}
// The second open should not cause a second call
expect(mockAxios.get).toHaveBeenCalledTimes(1);
closeModal(wrapper);

test('should display shortUrl if available', async () => {
// Changing the timetable should cause opening the modal to trigger another API call
wrapper.setProps({ timetable: { CS3216: { Lecture: '1' } } });
expect(mockAxios.get).toHaveBeenCalledTimes(1);
openModal(wrapper);
expect(mockAxios.get).toHaveBeenCalledTimes(2);
closeModal(wrapper);

// Changing the semester should also trigger another API call
wrapper.setProps({ semester: 2 });
expect(mockAxios.get).toHaveBeenCalledTimes(2);
openModal(wrapper);
expect(mockAxios.get).toHaveBeenCalledTimes(3);
});

test('should show spinner when loading', () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);

openModal(wrapper);
expect(wrapper.find(LoadingSpinner).exists()).toBe(true);
});

test('should display shortUrl with show original url button if available', async () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);

await openAndWait(wrapper);

if (enableShortUrl) {
expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL);
} else {
expect(wrapper.find('input').prop('value')).toBeTruthy();
}
expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL);
expect(wrapper.find(Maximize2).exists()).toBe(true);
});

test('should display long URL if data is corrupted', async () => {
Expand All @@ -122,6 +113,7 @@ describe('ShareTimetable', () => {

await openAndWait(wrapper);

expect(wrapper.find('button').at(1).prop('disabled')).toBe(true);
expect(wrapper.find('input').prop('value')).toBeTruthy();
});

Expand All @@ -133,6 +125,7 @@ describe('ShareTimetable', () => {

await openAndWait(wrapper);

expect(wrapper.find('button').at(1).prop('disabled')).toBe(true);
expect(wrapper.find('input').prop('value')).toBeTruthy();
});

Expand All @@ -155,4 +148,19 @@ describe('ShareTimetable', () => {

expect(wrapper.find('input').prop('value')).toContain('hidden=CS1010S,CS1231S');
});

test('should change to original url and display shorten url button when clicked on show original url button', async () => {
const wrapper = shallow(
<ShareTimetable semester={1} timetable={timetable} hiddenModules={[]} />,
);

await openAndWait(wrapper);
expect(wrapper.find('input').prop('value')).toEqual(MOCK_SHORTURL);
expect(wrapper.find(Maximize2).exists()).toBe(true);

wrapper.find('button').at(1).simulate('click');
expect(wrapper.find(Maximize2).exists()).toBe(false);
expect(wrapper.find(Minimize2).exists()).toBe(true);
expect(wrapper.find('input').prop('value')).not.toBe(MOCK_SHORTURL);
});
});
Loading

0 comments on commit e7452a7

Please sign in to comment.