From 6fede88072575d64f23090e6272a738b047f755d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 19:16:44 +0200 Subject: [PATCH 1/5] Added dependency on bowser to have a more accurate browser and OS detection --- package-lock.json | 5 +++ package.json | 1 + src/utils/helpers/visits.js | 48 ++++++++--------------- src/visits/GraphCard.js | 8 +++- test/visits/GraphCard.test.js | 8 +++- test/visits/services/VisitsParser.test.js | 4 +- 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/package-lock.json b/package-lock.json index 45ce16c2..bec22699 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3460,6 +3460,11 @@ "resolved": "https://registry.npmjs.org/bottlejs/-/bottlejs-1.7.2.tgz", "integrity": "sha512-voMPQ+g8/4GBiDE5lp43fgfoBn7Q5fZI7k3ye8ErrPoHzuuRS0Gff9lZYeyalh86uz5P304iZ14wNAM+3TZguQ==" }, + "bowser": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/bowser/-/bowser-2.9.0.tgz", + "integrity": "sha512-2ld76tuLBNFekRgmJfT2+3j5MIrP6bFict8WAIT3beq+srz1gcKNAdNKMqHqauQt63NmAa88HfP1/Ypa9Er3HA==" + }, "boxen": { "version": "1.3.0", "resolved": "https://registry.yarnpkg.com/boxen/-/boxen-1.3.0.tgz", diff --git a/package.json b/package.json index 9041a0eb..669cf8fd 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "axios": "^0.19.0", "bootstrap": "^4.3.1", "bottlejs": "^1.7.2", + "bowser": "^2.9.0", "chart.js": "^2.8.0", "classnames": "^2.2.6", "compare-versions": "^3.5.1", diff --git a/src/utils/helpers/visits.js b/src/utils/helpers/visits.js index 754cbb19..9e018430 100644 --- a/src/utils/helpers/visits.js +++ b/src/utils/helpers/visits.js @@ -1,26 +1,27 @@ +import bowser from 'bowser'; import { hasValue } from '../utils'; const DEFAULT = 'Others'; +const BROWSERS_WHITELIST = [ + 'Android Browser', + 'Chrome', + 'Chromium', + 'Firefox', + 'Internet Explorer', + 'Microsoft Edge', + 'Opera', + 'Safari', + 'Samsung Internet for Android', + 'Vivaldi', + 'WeChat', +]; export const osFromUserAgent = (userAgent) => { if (!hasValue(userAgent)) { return DEFAULT; } - const lowerUserAgent = userAgent.toLowerCase(); - - switch (true) { - case lowerUserAgent.includes('linux'): - return 'Linux'; - case lowerUserAgent.includes('windows'): - return 'Windows'; - case lowerUserAgent.includes('mac'): - return 'MacOS'; - case lowerUserAgent.includes('mobi'): - return 'Mobile'; - default: - return DEFAULT; - } + return bowser.parse(userAgent).os.name || DEFAULT; }; export const browserFromUserAgent = (userAgent) => { @@ -28,24 +29,9 @@ export const browserFromUserAgent = (userAgent) => { return DEFAULT; } - const lowerUserAgent = userAgent.toLowerCase(); + const { name: browser } = bowser.parse(userAgent).browser; - switch (true) { - case lowerUserAgent.includes('opera') || lowerUserAgent.includes('opr'): - return 'Opera'; - case lowerUserAgent.includes('firefox'): - return 'Firefox'; - case lowerUserAgent.includes('chrome'): - return 'Chrome'; - case lowerUserAgent.includes('safari'): - return 'Safari'; - case lowerUserAgent.includes('edg'): - return 'Microsoft Edge'; - case lowerUserAgent.includes('msie'): - return 'Internet Explorer'; - default: - return DEFAULT; - } + return browser && BROWSERS_WHITELIST.includes(browser) ? browser : DEFAULT; }; export const extractDomain = (url) => { diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 7cccf30d..3a0e6ac1 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -24,12 +24,16 @@ const generateGraphData = (title, isBarChart, labels, data, highlightedData) => data, backgroundColor: isBarChart ? 'rgba(70, 150, 229, 0.4)' : [ '#97BBCD', - '#DCDCDC', '#F7464A', '#46BFBD', '#FDB45C', '#949FB1', - '#4D5360', + '#57A773', + '#414066', + '#08B2E3', + '#B6C454', + '#DCDCDC', + '#463730', ], borderColor: isBarChart ? 'rgba(70, 150, 229, 1)' : 'white', borderWidth: 2, diff --git a/test/visits/GraphCard.test.js b/test/visits/GraphCard.test.js index 545e8c22..278dd799 100644 --- a/test/visits/GraphCard.test.js +++ b/test/visits/GraphCard.test.js @@ -31,12 +31,16 @@ describe('', () => { expect(datasets).toHaveLength(1); expect(backgroundColor).toEqual([ '#97BBCD', - '#DCDCDC', '#F7464A', '#46BFBD', '#FDB45C', '#949FB1', - '#4D5360', + '#57A773', + '#414066', + '#08B2E3', + '#B6C454', + '#DCDCDC', + '#463730', ]); expect(borderColor).toEqual('white'); expect(legend).toEqual({ position: 'right' }); diff --git a/test/visits/services/VisitsParser.test.js b/test/visits/services/VisitsParser.test.js index 3538be10..b02720bf 100644 --- a/test/visits/services/VisitsParser.test.js +++ b/test/visits/services/VisitsParser.test.js @@ -56,7 +56,7 @@ describe('VisitsParser', () => { expect(os).toEqual({ Linux: 3, Windows: 1, - MacOS: 1, + macOS: 1, }); }); @@ -137,7 +137,7 @@ describe('VisitsParser', () => { }, { browser: 'Firefox', - os: 'MacOS', + os: 'macOS', referer: 'google.com', country: 'United States', city: 'New York', From faf5d0bf7b857a4752d54f4c81b7890a8e5a1ec7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 19:22:13 +0200 Subject: [PATCH 2/5] Unified function parsing user agent for browser and os --- src/utils/helpers/visits.js | 16 ++++------------ src/visits/services/VisitsParser.js | 26 +++++++++++++++----------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/utils/helpers/visits.js b/src/utils/helpers/visits.js index 9e018430..8f5b9e8d 100644 --- a/src/utils/helpers/visits.js +++ b/src/utils/helpers/visits.js @@ -16,22 +16,14 @@ const BROWSERS_WHITELIST = [ 'WeChat', ]; -export const osFromUserAgent = (userAgent) => { +export const parseUserAgent = (userAgent) => { if (!hasValue(userAgent)) { - return DEFAULT; + return { browser: DEFAULT, os: DEFAULT }; } - return bowser.parse(userAgent).os.name || DEFAULT; -}; + const { browser: { name: browser }, os: { name: os } } = bowser.parse(userAgent); -export const browserFromUserAgent = (userAgent) => { - if (!hasValue(userAgent)) { - return DEFAULT; - } - - const { name: browser } = bowser.parse(userAgent).browser; - - return browser && BROWSERS_WHITELIST.includes(browser) ? browser : DEFAULT; + return { os: os || DEFAULT, browser: browser && BROWSERS_WHITELIST.includes(browser) ? browser : DEFAULT }; }; export const extractDomain = (url) => { diff --git a/src/visits/services/VisitsParser.js b/src/visits/services/VisitsParser.js index 2ebd5262..5ab88f71 100644 --- a/src/visits/services/VisitsParser.js +++ b/src/visits/services/VisitsParser.js @@ -1,5 +1,5 @@ import { isNil, map } from 'ramda'; -import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../../utils/helpers/visits'; +import { extractDomain, parseUserAgent } from '../../utils/helpers/visits'; import { hasValue } from '../../utils/utils'; const visitHasProperty = (visit, propertyName) => !isNil(visit) && hasValue(visit[propertyName]); @@ -59,13 +59,17 @@ export const processStatsFromVisits = (normalizedVisits) => { os: {}, browsers: {}, referrers: {}, countries: {}, cities: {}, citiesForMap: {} } ); -export const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({ - date, - browser: browserFromUserAgent(userAgent), - os: osFromUserAgent(userAgent), - referer: extractDomain(referer), - country: (visitLocation && visitLocation.countryName) || 'Unknown', - city: (visitLocation && visitLocation.cityName) || 'Unknown', - latitude: visitLocation && visitLocation.latitude, - longitude: visitLocation && visitLocation.longitude, -})); +export const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => { + const { browser, os } = parseUserAgent(userAgent); + + return { + date, + browser, + os, + referer: extractDomain(referer), + country: (visitLocation && visitLocation.countryName) || 'Unknown', + city: (visitLocation && visitLocation.cityName) || 'Unknown', + latitude: visitLocation && visitLocation.latitude, + longitude: visitLocation && visitLocation.longitude, + }; +}); From e37fb1b4bd9ccaa610768888771f84753fc48d7a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 19:29:57 +0200 Subject: [PATCH 3/5] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c42fb4b8..283f67bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Fixed * [#243](https://github.com/shlinkio/shlink-web-client/issues/243) Fixed loading state and resetting on short URL creation form. +* [#239](https://github.com/shlinkio/shlink-web-client/issues/239) Fixed how user agents are parsed, reducing false results. ## 2.3.1 - 2020-02-08 From 7cf49d2c1a1d82577bdd4d263d24464c0498a5b6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 10 Apr 2020 19:47:42 +0200 Subject: [PATCH 4/5] Increased minimum charts height --- src/visits/GraphCard.js | 2 +- src/visits/ShortUrlVisits.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/visits/GraphCard.js b/src/visits/GraphCard.js index 3a0e6ac1..acd40d5c 100644 --- a/src/visits/GraphCard.js +++ b/src/visits/GraphCard.js @@ -88,7 +88,7 @@ const renderGraph = (title, isBarChart, stats, max, highlightedStats, onClick) = }), }; const graphData = generateGraphData(title, isBarChart, labels, data, highlightedData); - const height = isBarChart && labels.length > 20 ? labels.length * 8 : null; + const height = isBarChart && labels.length > 20 ? labels.length * 8 : 200; // Provide a key based on the height, so that every time the dataset changes, a new graph is rendered return ( diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 1a057279..0d279ae2 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -198,7 +198,7 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa
{showTableControls && ( - +