Merge pull request #485 from shlinkio/hotfix/v3.2.1

Release v3.2.1

CHANGELOG.md

@@ -4,6 +4,25 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [3.2.1] - 2021-09-12
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#478](https://github.com/shlinkio/shlink-web-client/pull/478) Fixed tags including special chars not being properly URL encoded before using them as query params.
+* [#480](https://github.com/shlinkio/shlink-web-client/pull/480) Fixed servers import on Chromium-based browsers when using windows.
+* [#482](https://github.com/shlinkio/shlink-web-client/pull/480) Fixed end date not being set to the end of the day when filtering visits using a "smart filter" (last 7 days, last 30 days, etc).
+
+
 ## [3.2.0] - 2021-07-12
 ### Added
 * [#433](https://github.com/shlinkio/shlink-web-client/pull/433) Added support to provide a default server to connect to via env vars:

src/servers/helpers/ImportServersBtn.tsx

@@ -7,7 +7,7 @@ type Ref<T> = RefObject<T> | MutableRefObject<T>;
 
 export interface ImportServersBtnProps {
   onImport?: () => void;
-  onImportError?: () => void;
+  onImportError?: (error: Error) => void;
 }
 
 interface ImportServersBtnConnectProps extends ImportServersBtnProps {

src/servers/services/ServersImporter.ts

@@ -1,29 +1,37 @@
 import { CsvJson } from 'csvjson';
 import { ServerData } from '../data';
 
-interface CsvFile extends File {
-  type: 'text/csv' | 'text/comma-separated-values' | 'application/csv';
-}
+const validateServer = (server: any): server is ServerData =>
+  typeof server.url === 'string' && typeof server.apiKey === 'string' && typeof server.name === 'string';
 
-const CSV_MIME_TYPES = [ 'text/csv', 'text/comma-separated-values', 'application/csv' ];
-const isCsv = (file?: File | null): file is CsvFile => !!file && CSV_MIME_TYPES.includes(file.type);
+const validateServers = (servers: any): servers is ServerData[] =>
+  Array.isArray(servers) && servers.every(validateServer);
 
 export default class ServersImporter {
-  public constructor(private readonly csvjson: CsvJson, private readonly fileReaderFactory: () => FileReader) {}
+  public constructor(private readonly csvJson: CsvJson, private readonly fileReaderFactory: () => FileReader) {}
 
   public readonly importServersFromFile = async (file?: File | null): Promise<ServerData[]> => {
-    if (!isCsv(file)) {
-      throw new Error('No file provided or file is not a CSV');
+    if (!file) {
+      throw new Error('No file provided');
     }
 
     const reader = this.fileReaderFactory();
 
-    return new Promise((resolve) => {
+    return new Promise((resolve, reject) => {
       reader.addEventListener('loadend', (e: ProgressEvent<FileReader>) => {
-        const content = e.target?.result?.toString() ?? '';
-        const servers = this.csvjson.toObject<ServerData>(content);
+        try {
+          // TODO Read as stream, otherwise, if the file is too big, this will block the browser tab
+          const content = e.target?.result?.toString() ?? '';
+          const servers = this.csvJson.toObject(content);
 
-        resolve(servers);
+          if (!validateServers(servers)) {
+            throw new Error('Provided file does not have the right format.');
+          }
+
+          resolve(servers);
+        } catch (e) {
+          reject(e);
+        }
       });
       reader.readAsText(file);
     });

src/short-urls/ShortUrlsList.tsx

@@ -70,7 +70,7 @@ const ShortUrlsList = (ShortUrlsTable: FC<ShortUrlsTableProps>) => boundToMercur
 
   useEffect(() => {
     const { tag } = parseQuery<{ tag?: string }>(location.search);
-    const tags = tag ? [ tag ] : shortUrlsListParams.tags;
+    const tags = tag ? [ decodeURIComponent(tag) ] : shortUrlsListParams.tags;
 
     refreshList({ page: match.params.page, tags, itemsPerPage: undefined });
 

src/tags/TagCard.tsx

@@ -30,7 +30,7 @@ const TagCard = (
   const [ isEditModalOpen, toggleEdit ] = useToggle();
 
   const serverId = isServerWithId(selectedServer) ? selectedServer.id : '';
-  const shortUrlsLink = `/server/${serverId}/list-short-urls/1?tag=${tag}`;
+  const shortUrlsLink = `/server/${serverId}/list-short-urls/1?tag=${encodeURIComponent(tag)}`;
 
   return (
     <Card className="tag-card">

src/utils/dates/types/index.ts

@@ -55,6 +55,7 @@ export const rangeOrIntervalToString = (range?: DateRange | DateInterval): strin
 };
 
 const startOfDaysAgo = (daysAgo: number) => startOfDay(subDays(new Date(), daysAgo));
+const endingToday = (startDate: Date): DateRange => ({ startDate, endDate: endOfDay(new Date()) });
 
 export const intervalToDateRange = (dateInterval?: DateInterval): DateRange => {
   if (!dateInterval) {
@@ -63,19 +64,19 @@ export const intervalToDateRange = (dateInterval?: DateInterval): DateRange => {
 
   switch (dateInterval) {
     case 'today':
-      return { startDate: startOfDay(new Date()), endDate: new Date() };
+      return endingToday(startOfDay(new Date()));
     case 'yesterday':
       return { startDate: startOfDaysAgo(1), endDate: endOfDay(subDays(new Date(), 1)) };
     case 'last7Days':
-      return { startDate: startOfDaysAgo(7), endDate: new Date() };
+      return endingToday(startOfDaysAgo(7));
     case 'last30Days':
-      return { startDate: startOfDaysAgo(30), endDate: new Date() };
+      return endingToday(startOfDaysAgo(30));
     case 'last90Days':
-      return { startDate: startOfDaysAgo(90), endDate: new Date() };
+      return endingToday(startOfDaysAgo(90));
     case 'last180days':
-      return { startDate: startOfDaysAgo(180), endDate: new Date() };
+      return endingToday(startOfDaysAgo(180));
     case 'last365Days':
-      return { startDate: startOfDaysAgo(365), endDate: new Date() };
+      return endingToday(startOfDaysAgo(365));
   }
 
   return {};

test/servers/services/ServersImporter.test.ts

@@ -21,23 +21,70 @@ describe('ServersImporter', () => {
   describe('importServersFromFile', () => {
     it('rejects with error if no file was provided', async () => {
       await expect(importer.importServersFromFile()).rejects.toEqual(
-        new Error('No file provided or file is not a CSV'),
+        new Error('No file provided'),
       );
     });
 
-    it('rejects with error if provided file is not a CSV', async () => {
-      await expect(importer.importServersFromFile(Mock.of<File>({ type: 'text/html' }))).rejects.toEqual(
-        new Error('No file provided or file is not a CSV'),
-      );
+    it('rejects with error if parsing the file fails', async () => {
+      const expectedError = new Error('Error parsing file');
+
+      toObject.mockImplementation(() => {
+        throw expectedError;
+      });
+
+      await expect(importer.importServersFromFile(Mock.of<File>({ type: 'text/html' }))).rejects.toEqual(expectedError);
     });
 
     it.each([
-      [ 'text/csv' ],
-      [ 'text/comma-separated-values' ],
-      [ 'application/csv' ],
-    ])('reads file when a CSV is provided', async (type) => {
-      await importer.importServersFromFile(Mock.of<File>({ type }));
+      [{}],
+      [ undefined ],
+      [[{ foo: 'bar' }]],
+      [
+        [
+          {
+            url: 1,
+            apiKey: 1,
+            name: 1,
+          },
+        ],
+      ],
+      [
+        [
+          {
+            url: 'foo',
+            apiKey: 'foo',
+            name: 'foo',
+          },
+          { bar: 'foo' },
+        ],
+      ],
+    ])('rejects with error if provided file does not parse to valid list of servers', async (parsedObject) => {
+      toObject.mockReturnValue(parsedObject);
 
+      await expect(importer.importServersFromFile(Mock.of<File>({ type: 'text/html' }))).rejects.toEqual(
+        new Error('Provided file does not have the right format.'),
+      );
+    });
+
+    it('reads file when a CSV containing valid servers is provided', async () => {
+      const expectedServers = [
+        {
+          url: 'foo',
+          apiKey: 'foo',
+          name: 'foo',
+        },
+        {
+          url: 'bar',
+          apiKey: 'bar',
+          name: 'bar',
+        },
+      ];
+
+      toObject.mockReturnValue(expectedServers);
+
+      const result = await importer.importServersFromFile(Mock.all<File>());
+
+      expect(result).toEqual(expectedServers);
       expect(readAsText).toHaveBeenCalledTimes(1);
       expect(toObject).toHaveBeenCalledTimes(1);
     });

test/tags/TagCard.test.tsx

@@ -14,30 +14,36 @@ describe('<TagCard />', () => {
   };
   const DeleteTagConfirmModal = jest.fn();
   const EditTagModal = jest.fn();
-
-  beforeEach(() => {
-    const TagCard = createTagCard(DeleteTagConfirmModal, EditTagModal, () => null, Mock.all<ColorGenerator>());
-
+  const TagCard = createTagCard(DeleteTagConfirmModal, EditTagModal, () => null, Mock.all<ColorGenerator>());
+  const createWrapper = (tag = 'ssr') => {
     wrapper = shallow(
       <TagCard
-        tag="ssr"
+        tag={tag}
         selectedServer={Mock.of<ReachableServer>({ id: '1' })}
         tagStats={tagStats}
         displayed={true}
         toggle={() => {}}
       />,
     );
-  });
+
+    return wrapper;
+  };
+
+  beforeEach(() => createWrapper());
 
   afterEach(() => wrapper.unmount());
   afterEach(jest.resetAllMocks);
 
-  it('shows a TagBullet and a link to the list filtering by the tag', () => {
+  it.each([
+    [ 'ssr', '/server/1/list-short-urls/1?tag=ssr' ],
+    [ 'ssr-&-foo', '/server/1/list-short-urls/1?tag=ssr-%26-foo' ],
+  ])('shows a TagBullet and a link to the list filtering by the tag', (tag, expectedLink) => {
+    const wrapper = createWrapper(tag);
     const links = wrapper.find(Link);
     const bullet = wrapper.find(TagBullet);
 
-    expect(links.at(0).prop('to')).toEqual('/server/1/list-short-urls/1?tag=ssr');
-    expect(bullet.prop('tag')).toEqual('ssr');
+    expect(links.at(0).prop('to')).toEqual(expectedLink);
+    expect(bullet.prop('tag')).toEqual(tag);
   });
 
   it('displays delete modal when delete btn is clicked', () => {