diff --git a/client/pages/item/_id/index.vue b/client/pages/item/_id/index.vue index 1d8f0f20b..22711eb17 100644 --- a/client/pages/item/_id/index.vue +++ b/client/pages/item/_id/index.vue @@ -73,6 +73,9 @@

{{ $strings.LabelFinished }} {{ $formatDate(userProgressFinishedAt, dateFormat) }}

{{ $getString('LabelTimeRemaining', [$elapsedPretty(userTimeRemaining)]) }}

{{ $strings.LabelStarted }} {{ $formatDate(userProgressStartedAt, dateFormat) }}

+ +

{{ $strings.LabelPreviouslyFinished }} {{ $formatDate(finishedDateHistory[finishedDateHistory.length - 1], dateFormat) }}

+
@@ -334,6 +337,19 @@ export default { userProgressFinishedAt() { return this.userMediaProgress ? this.userMediaProgress.finishedAt : 0 }, + finishedDateHistory() { + const finishedDates = this.userMediaProgress?.finishedDates || [] + // The most recent finish date is already shown as the "Finished" line when progress is 100% + if (this.progressPercent >= 1) return finishedDates.slice(0, -1) + return finishedDates + }, + finishedDateHistoryTooltip() { + return this.finishedDateHistory + .slice() + .reverse() + .map((date) => `${this.$strings.LabelFinished} ${this.$formatDate(date, this.dateFormat)}`) + .join('
') + }, streamLibraryItem() { return this.$store.state.streamLibraryItem }, diff --git a/client/strings/en-us.json b/client/strings/en-us.json index fb2bcb281..5ea906c2f 100644 --- a/client/strings/en-us.json +++ b/client/strings/en-us.json @@ -527,6 +527,7 @@ "LabelPort": "Port", "LabelPrefixesToIgnore": "Prefixes to Ignore (case insensitive)", "LabelPreventIndexing": "Prevent your feed from being indexed by iTunes and Google podcast directories", + "LabelPreviouslyFinished": "Previously finished", "LabelPrimaryEbook": "Primary ebook", "LabelProgress": "Progress", "LabelProvider": "Provider", diff --git a/server/migrations/changelog.md b/server/migrations/changelog.md index 0fcbe6754..b0108e998 100644 --- a/server/migrations/changelog.md +++ b/server/migrations/changelog.md @@ -16,3 +16,4 @@ Please add a record of every database migration that you create to this file. Th | v2.19.1 | v2.19.1-copy-title-to-library-items | Copies title and titleIgnorePrefix to the libraryItems table, creates update triggers and indices | | v2.19.4 | v2.19.4-improve-podcast-queries | Adds numEpisodes to podcasts, adds podcastId to mediaProgresses, copies podcast title to libraryItems | | v2.20.0 | v2.20.0-improve-author-sort-queries | Adds AuthorNames(FirstLast\|LastFirst) to libraryItems to improve author sort queries | +| v2.36.0 | v2.36.0-add-finished-dates | Adds finishedDates JSON column to mediaProgresses to keep a history of every finish event | diff --git a/server/migrations/v2.36.0-add-finished-dates.js b/server/migrations/v2.36.0-add-finished-dates.js new file mode 100644 index 000000000..bebc6eada --- /dev/null +++ b/server/migrations/v2.36.0-add-finished-dates.js @@ -0,0 +1,91 @@ +/** + * @typedef MigrationContext + * @property {import('sequelize').QueryInterface} queryInterface - a suquelize QueryInterface object. + * @property {import('../Logger')} logger - a Logger object. + * + * @typedef MigrationOptions + * @property {MigrationContext} context - an object containing the migration context. + */ + +const migrationVersion = '2.36.0' +const migrationName = `${migrationVersion}-add-finished-dates` +const loggerPrefix = `[${migrationVersion} migration]` + +/** + * This upward migration adds a finishedDates JSON column to the mediaProgresses table + * and populates it from the existing finishedAt value so that every finish event is kept + * as history (re-listens/re-reads append to this array). + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function up({ context: { queryInterface, logger } }) { + // Upwards migration script + logger.info(`${loggerPrefix} UPGRADE BEGIN: ${migrationName}`) + + const tableDescription = await queryInterface.describeTable('mediaProgresses') + if (!tableDescription.finishedDates) { + logger.info(`${loggerPrefix} adding column "finishedDates" to table "mediaProgresses"`) + await queryInterface.addColumn('mediaProgresses', 'finishedDates', { type: queryInterface.sequelize.Sequelize.JSON, allowNull: true }) + logger.info(`${loggerPrefix} added column "finishedDates" to table "mediaProgresses"`) + } else { + logger.info(`${loggerPrefix} column "finishedDates" already exists in table "mediaProgresses"`) + } + + await populateFinishedDates(queryInterface, logger) + + logger.info(`${loggerPrefix} UPGRADE END: ${migrationName}`) +} + +/** + * This downward migration removes the finishedDates column from the mediaProgresses table. + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function down({ context: { queryInterface, logger } }) { + // Downward migration script + logger.info(`${loggerPrefix} DOWNGRADE BEGIN: ${migrationName}`) + + const tableDescription = await queryInterface.describeTable('mediaProgresses') + if (tableDescription.finishedDates) { + logger.info(`${loggerPrefix} removing column "finishedDates" from table "mediaProgresses"`) + await queryInterface.removeColumn('mediaProgresses', 'finishedDates') + logger.info(`${loggerPrefix} removed column "finishedDates" from table "mediaProgresses"`) + } else { + logger.info(`${loggerPrefix} column "finishedDates" does not exist in table "mediaProgresses"`) + } + + logger.info(`${loggerPrefix} DOWNGRADE END: ${migrationName}`) +} + +/** + * Populates the finishedDates column with a single-element array containing the existing + * finishedAt timestamp (in milliseconds since epoch) for rows that are missing it. + * Idempotent: only touches rows where finishedDates is still null. + * + * @param {import('sequelize').QueryInterface} queryInterface - a suquelize QueryInterface object. + * @param {import('../Logger')} logger - a Logger object. + */ +async function populateFinishedDates(queryInterface, logger) { + const [rows] = await queryInterface.sequelize.query(`SELECT id, finishedAt FROM mediaProgresses WHERE finishedAt IS NOT NULL AND finishedDates IS NULL`) + logger.info(`${loggerPrefix} populating finishedDates column for ${rows.length} rows in mediaProgresses table`) + + await queryInterface.sequelize.transaction(async (transaction) => { + for (const row of rows) { + const finishedAtMs = new Date(row.finishedAt).valueOf() + if (isNaN(finishedAtMs)) { + logger.error(`${loggerPrefix} invalid finishedAt value "${row.finishedAt}" for mediaProgress ${row.id}, skipping`) + continue + } + await queryInterface.sequelize.query(`UPDATE mediaProgresses SET finishedDates = :finishedDates WHERE id = :id`, { + replacements: { finishedDates: JSON.stringify([finishedAtMs]), id: row.id }, + transaction + }) + } + }) + + logger.info(`${loggerPrefix} populated finishedDates column in mediaProgresses table`) +} + +module.exports = { up, down } diff --git a/server/models/MediaProgress.js b/server/models/MediaProgress.js index 9c0269a9e..13f054df4 100644 --- a/server/models/MediaProgress.js +++ b/server/models/MediaProgress.js @@ -26,6 +26,8 @@ class MediaProgress extends Model { this.ebookProgress /** @type {Date} */ this.finishedAt + /** @type {number[]} */ + this.finishedDates /** @type {Object} */ this.extraData /** @type {UUIDV4} */ @@ -71,6 +73,7 @@ class MediaProgress extends Model { ebookLocation: DataTypes.STRING, ebookProgress: DataTypes.FLOAT, finishedAt: DataTypes.DATE, + finishedDates: DataTypes.JSON, extraData: DataTypes.JSON, podcastId: DataTypes.UUID }, @@ -171,7 +174,8 @@ class MediaProgress extends Model { ebookProgress: this.ebookProgress, lastUpdate: this.updatedAt.valueOf(), startedAt: this.createdAt.valueOf(), - finishedAt: this.finishedAt?.valueOf() || null + finishedAt: this.finishedAt?.valueOf() || null, + finishedDates: this.finishedDates || [] } } @@ -181,6 +185,31 @@ class MediaProgress extends Model { return Math.max(0, Math.min(this.currentTime / this.duration, 1)) } + /** + * Append a finish event to the finishedDates history + * + * @param {Date|number|string} finishedAt + */ + addFinishedDate(finishedAt) { + const finishedAtMs = new Date(finishedAt).valueOf() + if (isNaN(finishedAtMs)) { + Logger.warn(`[MediaProgress] Invalid finishedAt date "${finishedAt}" not added to finishedDates (media item ${this.mediaItemId})`) + return + } + const finishedDates = Array.isArray(this.finishedDates) ? [...this.finishedDates] : [] + finishedDates.push(finishedAtMs) + this.finishedDates = finishedDates + } + + /** + * Remove the most recent finish event from the finishedDates history. + * Used when explicitly un-marking an item as finished. + */ + removeLastFinishedDate() { + if (!Array.isArray(this.finishedDates) || !this.finishedDates.length) return + this.finishedDates = this.finishedDates.slice(0, -1) + } + /** * Apply update to media progress * @@ -189,13 +218,18 @@ class MediaProgress extends Model { */ async applyProgressUpdate(progressPayload) { if (!this.extraData) this.extraData = {} + // finishedDates history is managed by the server + delete progressPayload.finishedDates if (progressPayload.isFinished !== undefined) { if (progressPayload.isFinished && !this.isFinished) { this.finishedAt = progressPayload.finishedAt || Date.now() + this.addFinishedDate(this.finishedAt) this.extraData.progress = 1 this.changed('extraData', true) delete progressPayload.finishedAt } else if (!progressPayload.isFinished && this.isFinished) { + // Explicitly un-marking as finished undoes the most recent finish event + this.removeLastFinishedDate() this.finishedAt = null this.extraData.progress = 0 this.currentTime = 0 @@ -240,9 +274,11 @@ class MediaProgress extends Model { if (!this.isFinished && shouldMarkAsFinished) { this.isFinished = true this.finishedAt = this.finishedAt || Date.now() + this.addFinishedDate(this.finishedAt) this.extraData.progress = 1 this.changed('extraData', true) } else if (this.isFinished && this.changed('currentTime') && !shouldMarkAsFinished) { + // Re-listening to a finished item resets isFinished but keeps the finishedDates history this.isFinished = false this.finishedAt = null } diff --git a/server/models/User.js b/server/models/User.js index f380f8e4f..4a2797418 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -810,6 +810,7 @@ class User extends Model { } if (newMediaProgressPayload.isFinished) { newMediaProgressPayload.finishedAt = newMediaProgressPayload.finishedAt || new Date() + newMediaProgressPayload.finishedDates = [new Date(newMediaProgressPayload.finishedAt).valueOf()] newMediaProgressPayload.extraData.progress = 1 } else { newMediaProgressPayload.finishedAt = null diff --git a/server/utils/queries/userStats.js b/server/utils/queries/userStats.js index fbba7129a..c6bd4a584 100644 --- a/server/utils/queries/userStats.js +++ b/server/utils/queries/userStats.js @@ -41,14 +41,22 @@ module.exports = { * @returns {Promise} */ async getBookMediaProgressFinishedForYear(userId, year) { + const yearStartMs = Date.UTC(year, 0, 1) + const yearEndMs = Date.UTC(year + 1, 0, 1) const progresses = await Database.mediaProgressModel.findAll({ where: { userId, mediaItemType: 'book', - finishedAt: { - [Sequelize.Op.gte]: `${year}-01-01`, - [Sequelize.Op.lt]: `${year + 1}-01-01` - } + [Sequelize.Op.or]: [ + { + finishedAt: { + [Sequelize.Op.gte]: `${year}-01-01`, + [Sequelize.Op.lt]: `${year + 1}-01-01` + } + }, + // Also include books finished during the year that were re-started or finished again later (finishedDates history) + Sequelize.literal(`EXISTS (SELECT 1 FROM json_each(COALESCE(\`mediaProgress\`.\`finishedDates\`, '[]')) WHERE json_each.value >= ${yearStartMs} AND json_each.value < ${yearEndMs})`) + ] }, include: { model: Database.bookModel, diff --git a/test/server/migrations/v2.36.0-add-finished-dates.test.js b/test/server/migrations/v2.36.0-add-finished-dates.test.js new file mode 100644 index 000000000..4cf3bf329 --- /dev/null +++ b/test/server/migrations/v2.36.0-add-finished-dates.test.js @@ -0,0 +1,95 @@ +const chai = require('chai') +const sinon = require('sinon') +const { expect } = chai + +const { DataTypes, Sequelize } = require('sequelize') +const Logger = require('../../../server/Logger') + +const { up, down } = require('../../../server/migrations/v2.36.0-add-finished-dates') + +describe('Migration v2.36.0-add-finished-dates', () => { + let sequelize + let queryInterface + + const finishedAt1 = new Date('2024-03-05T18:23:11.000Z') + const finishedAt2 = new Date('2025-11-30T08:00:00.000Z') + + beforeEach(async () => { + sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + queryInterface = sequelize.getQueryInterface() + sinon.stub(Logger, 'info') + sinon.stub(Logger, 'error') + + await queryInterface.createTable('mediaProgresses', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + userId: { type: DataTypes.INTEGER, allowNull: false }, + mediaItemId: { type: DataTypes.INTEGER, allowNull: false }, + mediaItemType: { type: DataTypes.STRING, allowNull: false }, + isFinished: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: false }, + finishedAt: { type: DataTypes.DATE, allowNull: true } + }) + + await queryInterface.bulkInsert('mediaProgresses', [ + { id: 1, userId: 1, mediaItemId: 1, mediaItemType: 'book', isFinished: 1, finishedAt: finishedAt1 }, + { id: 2, userId: 1, mediaItemId: 2, mediaItemType: 'book', isFinished: 0, finishedAt: null }, + { id: 3, userId: 2, mediaItemId: 1, mediaItemType: 'podcastEpisode', isFinished: 1, finishedAt: finishedAt2 } + ]) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('up', () => { + it('should add the finishedDates column and populate it from finishedAt', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const tableDescription = await queryInterface.describeTable('mediaProgresses') + expect(tableDescription.finishedDates).to.exist + + const [rows] = await sequelize.query('SELECT id, finishedDates FROM mediaProgresses ORDER BY id') + expect(JSON.parse(rows[0].finishedDates)).to.deep.equal([finishedAt1.valueOf()]) + expect(rows[1].finishedDates).to.be.null + expect(JSON.parse(rows[2].finishedDates)).to.deep.equal([finishedAt2.valueOf()]) + }) + + it('should be idempotent', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await up({ context: { queryInterface, logger: Logger } }) + + const [rows] = await sequelize.query('SELECT id, finishedDates FROM mediaProgresses ORDER BY id') + expect(JSON.parse(rows[0].finishedDates)).to.deep.equal([finishedAt1.valueOf()]) + expect(rows[1].finishedDates).to.be.null + expect(JSON.parse(rows[2].finishedDates)).to.deep.equal([finishedAt2.valueOf()]) + }) + + it('should not overwrite an existing finishedDates value', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + await sequelize.query(`UPDATE mediaProgresses SET finishedDates = '[1000,2000]' WHERE id = 1`) + await up({ context: { queryInterface, logger: Logger } }) + + const [rows] = await sequelize.query('SELECT finishedDates FROM mediaProgresses WHERE id = 1') + expect(JSON.parse(rows[0].finishedDates)).to.deep.equal([1000, 2000]) + }) + }) + + describe('down', () => { + it('should remove the finishedDates column', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const tableDescription = await queryInterface.describeTable('mediaProgresses') + expect(tableDescription.finishedDates).to.not.exist + }) + + it('should be idempotent', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const tableDescription = await queryInterface.describeTable('mediaProgresses') + expect(tableDescription.finishedDates).to.not.exist + }) + }) +})