From 32b76303e5ee8587373a0e6b65a49def243109d1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 31 Aug 2022 08:16:40 +0200 Subject: [PATCH] Add simple count of views to share links (#4036) * Add simple count of views to share links * Remove no longer applicable tests * Avoid incrementing view count for known bots --- package.json | 3 +++ server/commands/documentLoader.ts | 8 ------ .../20220830215146-add-shares-views.js | 13 ++++++++++ server/models/Share.ts | 6 +++++ server/presenters/share.ts | 1 + server/routes/api/documents.test.ts | 8 ------ server/routes/app.ts | 8 ++++++ server/routes/index.ts | 5 +++- yarn.lock | 26 +++++++++++++++++++ 9 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 server/migrations/20220830215146-add-shares-views.js diff --git a/package.json b/package.json index 82260c45c..366ec8ac9 100644 --- a/package.json +++ b/package.json @@ -124,6 +124,7 @@ "koa-send": "5.0.1", "koa-sslify": "2.1.2", "koa-static": "^4.0.1", + "koa-useragent": "^4.1.0", "lodash": "^4.17.21", "mammoth": "^1.4.19", "markdown-it": "^13.0.1", @@ -231,6 +232,7 @@ "@types/emoji-regex": "^9.2.0", "@types/enzyme": "^3.10.10", "@types/enzyme-adapter-react-16": "^1.0.6", + "@types/express-useragent": "^1.0.2", "@types/formidable": "^2.0.5", "@types/fs-extra": "^9.0.13", "@types/fuzzy-search": "^2.1.2", @@ -247,6 +249,7 @@ "@types/koa-router": "^7.4.4", "@types/koa-sslify": "^2.1.0", "@types/koa-static": "^4.0.2", + "@types/koa-useragent": "^2.1.2", "@types/markdown-it": "^12.2.3", "@types/markdown-it-container": "^2.0.4", "@types/markdown-it-emoji": "^2.0.2", diff --git a/server/commands/documentLoader.ts b/server/commands/documentLoader.ts index b0ecd609a..8a3767a78 100644 --- a/server/commands/documentLoader.ts +++ b/server/commands/documentLoader.ts @@ -97,10 +97,6 @@ export default async function loadDocument({ const canReadDocument = user && can(user, "read", document); if (canReadDocument) { - await share.update({ - lastAccessedAt: new Date(), - }); - // Cannot use document.collection here as it does not include the // documentStructure by default through the relationship. collection = await Collection.findByPk(document.collectionId); @@ -156,10 +152,6 @@ export default async function loadDocument({ if (!team.sharing) { throw AuthorizationError(); } - - await share.update({ - lastAccessedAt: new Date(), - }); } else { document = await Document.findByPk(id as string, { userId: user ? user.id : undefined, diff --git a/server/migrations/20220830215146-add-shares-views.js b/server/migrations/20220830215146-add-shares-views.js new file mode 100644 index 000000000..4d537a7bb --- /dev/null +++ b/server/migrations/20220830215146-add-shares-views.js @@ -0,0 +1,13 @@ +'use strict'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn("shares", "views", { + type: Sequelize.INTEGER, + defaultValue: 0 + }); + }, + down: async (queryInterface) => { + await queryInterface.removeColumn("shares", "views"); + }, +}; diff --git a/server/models/Share.ts b/server/models/Share.ts index 1db4bd0ab..050c4938f 100644 --- a/server/models/Share.ts +++ b/server/models/Share.ts @@ -6,6 +6,7 @@ import { Table, Scopes, DataType, + Default, } from "sequelize-typescript"; import Collection from "./Collection"; import Document from "./Document"; @@ -79,6 +80,11 @@ class Share extends IdModel { @Column lastAccessedAt: Date | null; + /** Total count of times the shared link has been accessed */ + @Default(0) + @Column + views: number; + // getters get isRevoked() { diff --git a/server/presenters/share.ts b/server/presenters/share.ts index 444d28dc1..067ae2553 100644 --- a/server/presenters/share.ts +++ b/server/presenters/share.ts @@ -12,6 +12,7 @@ export default function present(share: Share, isAdmin = false) { createdBy: presentUser(share.user), includeChildDocuments: share.includeChildDocuments, lastAccessedAt: share.lastAccessedAt || undefined, + views: share.views || 0, createdAt: share.createdAt, updatedAt: share.updatedAt, }; diff --git a/server/routes/api/documents.test.ts b/server/routes/api/documents.test.ts index b87eafb43..35753e965 100644 --- a/server/routes/api/documents.test.ts +++ b/server/routes/api/documents.test.ts @@ -102,8 +102,6 @@ describe("#documents.info", () => { expect(body.data.id).toEqual(document.id); expect(body.data.createdBy).toEqual(undefined); expect(body.data.updatedBy).toEqual(undefined); - await share.reload(); - expect(share.lastAccessedAt).toBeTruthy(); }); it("should not return document of a deleted collection, when the user was absent in the collection", async () => { @@ -192,8 +190,6 @@ describe("#documents.info", () => { expect(body.data.document.createdBy).toEqual(undefined); expect(body.data.document.updatedBy).toEqual(undefined); expect(body.data.sharedTree).toEqual(collection.documentStructure?.[0]); - await share.reload(); - expect(share.lastAccessedAt).toBeTruthy(); }); it("should return sharedTree from shareId with id of nested document", async () => { const { document, user } = await seed(); @@ -215,8 +211,6 @@ describe("#documents.info", () => { expect(body.data.document.createdBy).toEqual(undefined); expect(body.data.document.updatedBy).toEqual(undefined); expect(body.data.sharedTree).toEqual(document.toJSON()); - await share.reload(); - expect(share.lastAccessedAt).toBeTruthy(); }); it("should not return sharedTree if child documents not shared", async () => { const { document, user } = await seed(); @@ -238,8 +232,6 @@ describe("#documents.info", () => { expect(body.data.document.createdBy).toEqual(undefined); expect(body.data.document.updatedBy).toEqual(undefined); expect(body.data.sharedTree).toEqual(undefined); - await share.reload(); - expect(share.lastAccessedAt).toBeTruthy(); }); it("should not return details for nested documents", async () => { const { document, collection, user } = await seed(); diff --git a/server/routes/app.ts b/server/routes/app.ts index a63d0fc8d..d192b2e48 100644 --- a/server/routes/app.ts +++ b/server/routes/app.ts @@ -3,6 +3,7 @@ import path from "path"; import util from "util"; import { Context, Next } from "koa"; import { escape } from "lodash"; +import { Sequelize } from "sequelize"; import documentLoader from "@server/commands/documentLoader"; import env from "@server/env"; import presentEnv from "@server/presenters/env"; @@ -81,6 +82,13 @@ export const renderShare = async (ctx: Context, next: Next) => { }); share = result.share; document = result.document; + + if (share && !ctx.userAgent.isBot) { + await share.update({ + lastAccessedAt: new Date(), + views: Sequelize.literal("views + 1"), + }); + } } catch (err) { // If the share or document does not exist, return a 404. ctx.status = 404; diff --git a/server/routes/index.ts b/server/routes/index.ts index fbddbf9bb..73af3322a 100644 --- a/server/routes/index.ts +++ b/server/routes/index.ts @@ -1,8 +1,9 @@ import path from "path"; -import Koa from "koa"; +import Koa, { BaseContext } from "koa"; import Router from "koa-router"; import send from "koa-send"; import serve from "koa-static"; +import userAgent, { UserAgentContext } from "koa-useragent"; import { languages } from "@shared/i18n"; import env from "@server/env"; import { NotFoundError } from "@server/errors"; @@ -22,6 +23,8 @@ koa.use( }) ); +koa.use(userAgent); + if (isProduction) { router.get("/static/*", async (ctx) => { try { diff --git a/yarn.lock b/yarn.lock index 0d7c261c1..fa549bfb0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2601,6 +2601,13 @@ "@types/qs" "*" "@types/range-parser" "*" +"@types/express-useragent@^1.0.2": + version "1.0.2" + resolved "https://registry.yarnpkg.com/@types/express-useragent/-/express-useragent-1.0.2.tgz#c129f2aae983fb7b646dbcd254973936f745c12f" + integrity sha512-eUVCqMsmEO7adMJSxuAARPUxbEJLYQJATiB86bx3MGeyUOTgKNnLTfAMaF+z84DftcH6NBbFFwiRomIcsFVdUQ== + dependencies: + "@types/express" "*" + "@types/express@*": version "4.17.13" resolved "https://registry.yarnpkg.com/@types/express/-/express-4.17.13.tgz#a76e2995728999bab51a33fabce1d705a3709034" @@ -2821,6 +2828,13 @@ "@types/koa" "*" "@types/koa-send" "*" +"@types/koa-useragent@^2.1.2": + version "2.1.2" + resolved "https://registry.yarnpkg.com/@types/koa-useragent/-/koa-useragent-2.1.2.tgz#7c75fe55c742e559c4643d65b34c6ce5945f853f" + integrity sha512-vpOSl6Rw6aCJr+kyWb27kGOdFiQD5WQeLOOOQgwMY9Lrqwbocm/td5paP5uE8bOy58ik/rZUly8zoVZACwZXUA== + dependencies: + koa-useragent "*" + "@types/koa@*", "@types/koa@^2.13.1", "@types/koa@^2.13.4": version "2.13.4" resolved "https://registry.yarnpkg.com/@types/koa/-/koa-2.13.4.tgz#10620b3f24a8027ef5cbae88b393d1b31205726b" @@ -7204,6 +7218,11 @@ exports-loader@^0.6.4: loader-utils "^1.0.2" source-map "0.5.x" +express-useragent@^1.0.15: + version "1.0.15" + resolved "https://registry.yarnpkg.com/express-useragent/-/express-useragent-1.0.15.tgz#cefda5fa4904345d51d3368b117a8dd4124985d9" + integrity sha512-eq5xMiYCYwFPoekffMjvEIk+NWdlQY9Y38OsTyl13IvA728vKT+q/CSERYWzcw93HGBJcIqMIsZC5CZGARPVdg== + ext@^1.1.2: version "1.4.0" resolved "https://registry.yarnpkg.com/ext/-/ext-1.4.0.tgz#89ae7a07158f79d35517882904324077e4379244" @@ -9969,6 +9988,13 @@ koa-static@^5.0.0: debug "^3.1.0" koa-send "^5.0.0" +koa-useragent@*, koa-useragent@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/koa-useragent/-/koa-useragent-4.1.0.tgz#d3f128b552c6da3e5e9e9e9c887b2922b16e4468" + integrity sha512-x/HUDZ1zAmNNh5hA9hHbPm9p3UVg2prlpHzxCXQCzbibrNS0kmj7MkCResCbAbG7ZT6FVxNSMjR94ZGamdMwxA== + dependencies: + express-useragent "^1.0.15" + koa-views@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/koa-views/-/koa-views-7.0.1.tgz#0c8f8e65d5cd2e08249430cb83dc361e49a17a5a"