diff --git a/docs/data-model.md b/docs/data-model.md index 8f7b38fc21..3e5278a837 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -184,7 +184,7 @@ Concerning the **Update mode** of the fields: | `text` | Tag name. | `TPC`, `COSMICS`, `RC` | | `id` | Insert | | `Mattermost` | Mattermost channels | `Food`, `Bookkeeping updates` | | `id` | Update | | `email` | Email groups | `food@cern.ch`, `Bookkeeping-updates@cern.ch` | | `id` | Update | -| `last_edited_name` | Name of the person who last edited the email/mattermost fields | `Anonymous`, `Jan Janssen` | When email/mattermost is edited | `id` | Update | +| `last_edited_by_user_id` | Id (in `users` table) of the user who last edited the tag | `1`, `2` | When the tag is edited | `id` | Update | ## Environments diff --git a/lib/database/adapters/TagAdapter.js b/lib/database/adapters/TagAdapter.js index 5f44a81e26..af8ad5affa 100644 --- a/lib/database/adapters/TagAdapter.js +++ b/lib/database/adapters/TagAdapter.js @@ -21,6 +21,7 @@ class TagAdapter { constructor() { this.toEntity = this.toEntity.bind(this); this.toDatabase = this.toDatabase.bind(this); + this.userAdapter = null; } /** @@ -29,12 +30,12 @@ class TagAdapter { * @param {SequelizeTag} databaseObject Object to convert. * @returns {Tag} Converted entity object. */ - toEntity({ id, text, description, email, mattermost, last_edited_name, archived, color, archivedAt, updatedAt }) { + toEntity({ id, text, description, email, mattermost, lastEditedBy, archived, color, archivedAt, updatedAt }) { return { id, text, description, - lastEditedName: last_edited_name, + lastEditedBy: lastEditedBy ? this.userAdapter.toNameOnly(lastEditedBy) : null, email, mattermost, archived, diff --git a/lib/database/adapters/index.js b/lib/database/adapters/index.js index 5ff6404b3d..fc9a81a696 100644 --- a/lib/database/adapters/index.js +++ b/lib/database/adapters/index.js @@ -139,6 +139,8 @@ runAdapter.logAdapter = logAdapter; runAdapter.runTypeAdapter = runTypeAdapter; runAdapter.tagAdapter = tagAdapter; runAdapter.userAdapter = userAdapter; + +tagAdapter.userAdapter = userAdapter; runAdapter.qcFlagAdapter = qcFlagAdapter; simulationPassQcFlagAdapter.simulationPassAdapter = simulationPassAdapter; diff --git a/lib/database/migrations/v1/20260930100000-tags-last-edited-by-user-id.js b/lib/database/migrations/v1/20260930100000-tags-last-edited-by-user-id.js new file mode 100644 index 0000000000..2f0e28b514 --- /dev/null +++ b/lib/database/migrations/v1/20260930100000-tags-last-edited-by-user-id.js @@ -0,0 +1,56 @@ +/* + * @license + * Copyright CERN and copyright holders of ALICE O2. This software is + * distributed under the terms of the GNU General Public License v3 (GPL + * Version 3), copied verbatim in the file "COPYING". + * + * See http://alice-o2.web.cern.ch/license for full licensing information. + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +'use strict'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + up: async (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.addColumn('tags', 'last_edited_by_user_id', { + type: Sequelize.INTEGER, + allowNull: true, + references: { + model: 'users', + key: 'id', + }, + onUpdate: 'CASCADE', + onDelete: 'SET NULL', + }, { transaction }); + + // Link existing tags to the user matching the stored name (ambiguous names resolve to the oldest user) + await queryInterface.sequelize.query( + `UPDATE tags t + SET t.last_edited_by_user_id = (SELECT MIN(u.id) FROM users u WHERE u.name = t.last_edited_name) + WHERE t.last_edited_name IS NOT NULL`, + { transaction }, + ); + + await queryInterface.removeColumn('tags', 'last_edited_name', { transaction }); + }), + + down: async (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.addColumn('tags', 'last_edited_name', { + type: Sequelize.STRING, + allowNull: true, + }, { transaction }); + + await queryInterface.sequelize.query( + `UPDATE tags t + INNER JOIN users u ON u.id = t.last_edited_by_user_id + SET t.last_edited_name = u.name`, + { transaction }, + ); + + await queryInterface.removeColumn('tags', 'last_edited_by_user_id', { transaction }); + }), +}; diff --git a/lib/database/models/tag.js b/lib/database/models/tag.js index 7bacf4a9b4..b6165c7a4f 100644 --- a/lib/database/models/tag.js +++ b/lib/database/models/tag.js @@ -31,8 +31,8 @@ module.exports = (sequelize) => { type: Sequelize.STRING, allowNull: true, }, - last_edited_name: { - type: Sequelize.STRING, + lastEditedByUserId: { + type: Sequelize.INTEGER, allowNull: true, }, archivedAt: { @@ -66,6 +66,7 @@ module.exports = (sequelize) => { Tag.associate = (models) => { Tag.belongsToMany(models.Log, { through: 'log_tags' }); Tag.belongsToMany(models.Run, { through: 'run_tags', as: 'runs' }); + Tag.belongsTo(models.User, { as: 'lastEditedBy', foreignKey: 'lastEditedByUserId' }); }; return Tag; diff --git a/lib/database/models/typedefs/SequelizeTag.js b/lib/database/models/typedefs/SequelizeTag.js index 6257015d9c..35477c301a 100644 --- a/lib/database/models/typedefs/SequelizeTag.js +++ b/lib/database/models/typedefs/SequelizeTag.js @@ -19,7 +19,7 @@ * @property {string} description * @property {string|null} email * @property {string|null} mattermost - * @property {string|null} last_edited_name + * @property {SequelizeUser|null} [lastEditedBy] * @property {string|null} archivedAt * @property {string} createdAt * @property {string} updatedAt diff --git a/lib/domain/entities/Tag.js b/lib/domain/entities/Tag.js index f19448a94c..3aa8ba76a5 100644 --- a/lib/domain/entities/Tag.js +++ b/lib/domain/entities/Tag.js @@ -18,7 +18,7 @@ * @property {string} description * @property {string|null} email * @property {string|null} mattermost - * @property {string|null} lastEditedName + * @property {{name: string}|null} lastEditedBy the name of the user who last edited the tag * @property {boolean} archived * @property {number|null} archivedAt * @property {number} createdAt diff --git a/lib/public/components/tag/tagDetail.js b/lib/public/components/tag/tagDetail.js index 83d9190e5e..b89f60995d 100644 --- a/lib/public/components/tag/tagDetail.js +++ b/lib/public/components/tag/tagDetail.js @@ -78,11 +78,11 @@ const activeFields = (detailsModel) => ({ size: 'cell-m', format: (timestamp) => formatTimestamp(timestamp), }, - lastEditedName: { + lastEditedBy: { name: 'Last modified by', visible: true, size: 'cell-m', - format: (name) => name ? name : '-', + format: (user) => user?.name || '-', }, archived: { name: 'Archived', diff --git a/lib/public/views/Tags/ActiveColumns/tagsActiveColumns.js b/lib/public/views/Tags/ActiveColumns/tagsActiveColumns.js index 8d5cb05c43..4551c2757a 100644 --- a/lib/public/views/Tags/ActiveColumns/tagsActiveColumns.js +++ b/lib/public/views/Tags/ActiveColumns/tagsActiveColumns.js @@ -36,11 +36,11 @@ export const tagsActiveColumns = { placeholder: 'Filter by name', }, }, - lastEditedName: { + lastEditedBy: { name: 'Last Edited by', visible: true, classes: 'w-10 f6', - format: (name) => name || '-', + format: (user) => user?.name || '-', }, updatedAt: { name: 'Updated at', diff --git a/lib/server/controllers/tags.controller.js b/lib/server/controllers/tags.controller.js index 76c480e629..553f2db9d5 100644 --- a/lib/server/controllers/tags.controller.js +++ b/lib/server/controllers/tags.controller.js @@ -24,6 +24,7 @@ const { } = require('../../usecases'); const { dtos: { CreateTagDto, GetAllTagsDto, GetTagDto, UpdateTagDto, GetTagByNameDto } } = require('../../domain'); const { dtoValidator } = require('../utilities'); +const { updateExpressResponseFromNativeError } = require('../express/updateExpressResponseFromNativeError.js'); const { ApiConfig } = require('../../config/index.js'); const GetAllLogsByTagDto = require('../../domain/dtos/GetAllLogsByTagDto.js'); @@ -42,23 +43,11 @@ const createTag = async (request, response) => { return; } - const tag = await new CreateTagUseCase().execute(value); - - if (tag) { - response.status(201).json({ - data: tag, - }); - } else { - response.status(409).json({ - errors: [ - { - status: '409', - source: { pointer: '/data/attributes/body/text' }, - title: 'Conflict', - detail: 'The provided entity already exists', - }, - ], - }); + try { + const tag = await new CreateTagUseCase().execute(value); + response.status(201).json({ data: tag }); + } catch (error) { + updateExpressResponseFromNativeError(response, error); } }; @@ -289,13 +278,11 @@ const updateTagById = async (request, response) => { return; } - const { result, error } = await new UpdateTagUseCase() - .execute(value); - - if (error) { - response.status(Number(error.status)).json({ errors: [error] }); - } else { - response.status(201).json({ data: result }); + try { + const tag = await new UpdateTagUseCase().execute(value); + response.status(201).json({ data: tag }); + } catch (error) { + updateExpressResponseFromNativeError(response, error); } }; diff --git a/lib/usecases/tag/CreateTagUseCase.js b/lib/usecases/tag/CreateTagUseCase.js index 7d2ddb4f4f..aadc1d4529 100644 --- a/lib/usecases/tag/CreateTagUseCase.js +++ b/lib/usecases/tag/CreateTagUseCase.js @@ -21,6 +21,9 @@ const { }, } = require('../../database'); const { tagAdapter } = require('../../database/adapters/index.js'); +const { BadParameterError } = require('../../server/errors/BadParameterError.js'); +const { ConflictError } = require('../../server/errors/ConflictError.js'); +const { getUserOrFail } = require('../../server/services/user/getUserOrFail.js'); /** * CreateTagUseCase @@ -30,23 +33,31 @@ class CreateTagUseCase { * Executes this use case. * * @param {Object} dto The CreateTagDto containing all data. - * @returns {Promise} Promise object represents the result of this use case. + * @returns {Promise} resolves with the created tag + * @throws {BadParameterError} if no user is provided in the session + * @throws {NotFoundError} if the session user does not exist + * @throws {ConflictError} if a tag with the same text already exists */ async execute(dto) { const { body } = dto; - body.last_edited_name = dto?.session?.name; + const userId = dto?.session?.id; + if (userId === undefined || userId === null) { + throw new BadParameterError('A user is required to create a tag'); + } + const tag = await TransactionHelper.provide(async () => { - const queryBuilder = new QueryBuilder() - .where('text').is(body.text); - const tag = await TagRepository.findOne(queryBuilder); - if (tag) { - return null; + const user = await getUserOrFail({ userId }); + body.lastEditedByUserId = user.id; + + const existingTag = await TagRepository.findOne(new QueryBuilder().where('text').is(body.text)); + if (existingTag) { + throw new ConflictError('The provided entity already exists'); } return TagRepository.insert(tagAdapter.toDatabase(body)); }); - return tag ? tagAdapter.toEntity(tag) : null; + return tagAdapter.toEntity(tag); } } diff --git a/lib/usecases/tag/GetAllTagsUseCase.js b/lib/usecases/tag/GetAllTagsUseCase.js index c23fe653da..fae6ea973b 100644 --- a/lib/usecases/tag/GetAllTagsUseCase.js +++ b/lib/usecases/tag/GetAllTagsUseCase.js @@ -33,7 +33,7 @@ class GetAllTagsUseCase { * @returns {Promise} Promise object represents the result of this use case. */ async execute(dto = {}) { - const queryBuilder = new QueryBuilder(); + const queryBuilder = new QueryBuilder().include({ association: 'lastEditedBy', attributes: ['name'] }); const { query = {} } = dto; const { filter = {} } = query; diff --git a/lib/usecases/tag/GetTagByNameUseCase.js b/lib/usecases/tag/GetTagByNameUseCase.js index 029fc40d3e..56b12591dd 100644 --- a/lib/usecases/tag/GetTagByNameUseCase.js +++ b/lib/usecases/tag/GetTagByNameUseCase.js @@ -38,7 +38,8 @@ class GetTagByNameUseCase { name = decodeURIComponent(name); const tag = await TransactionHelper.provide(async () => { const queryBuilder = new QueryBuilder() - .where('text').is(name); + .where('text').is(name) + .include({ association: 'lastEditedBy', attributes: ['name'] }); return TagRepository.findOne(queryBuilder); }); diff --git a/lib/usecases/tag/GetTagUseCase.js b/lib/usecases/tag/GetTagUseCase.js index 181c5f70c0..b2bc9bb604 100644 --- a/lib/usecases/tag/GetTagUseCase.js +++ b/lib/usecases/tag/GetTagUseCase.js @@ -38,7 +38,8 @@ class GetTagUseCase { const tag = await TransactionHelper.provide(async () => { const queryBuilder = new QueryBuilder() - .where('id').is(tagId); + .where('id').is(tagId) + .include({ association: 'lastEditedBy', attributes: ['name'] }); return TagRepository.findOne(queryBuilder); }); diff --git a/lib/usecases/tag/UpdateTagUseCase.js b/lib/usecases/tag/UpdateTagUseCase.js index 4b88e98c36..b1017674f5 100644 --- a/lib/usecases/tag/UpdateTagUseCase.js +++ b/lib/usecases/tag/UpdateTagUseCase.js @@ -21,6 +21,9 @@ const { }, } = require('../../database'); const GetTagUseCase = require('./GetTagUseCase'); +const { BadParameterError } = require('../../server/errors/BadParameterError.js'); +const { NotFoundError } = require('../../server/errors/NotFoundError.js'); +const { getUserOrFail } = require('../../server/services/user/getUserOrFail.js'); /** * Update tag use case @@ -28,40 +31,39 @@ const GetTagUseCase = require('./GetTagUseCase'); class UpdateTagUseCase { /** * Executes this use case + * * @param {Object} dto The UpdateTagDto containing the values that needs to be updated. - * @returns {Promise} Promise object that represents the result of the update. + * @returns {Promise} resolves with the updated tag + * @throws {BadParameterError} if no user is provided in the session + * @throws {NotFoundError} if the tag or the session user does not exist */ async execute(dto) { const { body, params } = dto; const { tagId } = params; const { description, email, mattermost, archivedAt, color } = body; - const tag = await TransactionHelper.provide(async () => { - const username = dto?.session?.name; - const queryBuilder = new QueryBuilder().where('id').is(tagId); - const tagObject = await TagRepository.findOne(queryBuilder); - if (tagObject && username) { - tagObject.description = description; - tagObject.email = email; - tagObject.color = color; - tagObject.mattermost = mattermost; - tagObject.archivedAt = archivedAt; - tagObject.last_edited_name = username; - await tagObject.save(); - return tagObject; - } else { - return { - error: { - status: '400', - title: `this tag with this tag id: (${tagId}) could not be found.`, - }, - }; + const userId = dto?.session?.id; + if (userId === undefined || userId === null) { + throw new BadParameterError('A user is required to update a tag'); + } + + await TransactionHelper.provide(async () => { + const tag = await TagRepository.findOne(new QueryBuilder().where('id').is(tagId)); + if (!tag) { + throw new NotFoundError(`Tag with this id (${tagId}) could not be found`); } + + const user = await getUserOrFail({ userId }); + + tag.description = description; + tag.email = email; + tag.color = color; + tag.mattermost = mattermost; + tag.archivedAt = archivedAt; + tag.lastEditedByUserId = user.id; + await tag.save(); }); - if (tag.error) { - return tag; - } - const result = await new GetTagUseCase().execute({ params: { tagId } }); - return { result }; + + return new GetTagUseCase().execute({ params: { tagId } }); } } diff --git a/proto/log.proto b/proto/log.proto index e035ff91ce..c97cf4d447 100644 --- a/proto/log.proto +++ b/proto/log.proto @@ -87,8 +87,8 @@ message Tag { string mattermost = 5; // Unix timestamp when this entity was last updated. int64 updatedAt = 6; - // The last person that edited the email/mattermost fields - string lastEditedName = 7; + reserved 7; + reserved "lastEditedName"; // The description of the tag optional string description = 8; } diff --git a/test/api/logs.test.js b/test/api/logs.test.js index 9d2f774ad3..a8e9fde03f 100644 --- a/test/api/logs.test.js +++ b/test/api/logs.test.js @@ -1224,7 +1224,7 @@ module.exports = () => { expect(response.body.data.text).to.equal('Text of yet another run'); for (const tag of response.body.data.tags) { delete tag.updatedAt; - delete tag.lastEditedName; + delete tag.lastEditedBy; } expect(response.body.data.tags).to.deep.equal([ { diff --git a/test/api/tags.test.js b/test/api/tags.test.js index 096a5a3606..f657a14ce9 100644 --- a/test/api/tags.test.js +++ b/test/api/tags.test.js @@ -312,6 +312,7 @@ module.exports = () => { }, }); + createTagDto.session = { id: 1, externalId: 1, name: 'John Doe' }; createdTag = await new CreateTagUseCase() .execute(createTagDto); }); @@ -543,6 +544,7 @@ module.exports = () => { text: `TAG#${new Date().getTime()}`, }, }); + createTagDto.session = { id: 1, externalId: 1, name: 'John Doe' }; createdTag = await new CreateTagUseCase() .execute(createTagDto); }); @@ -580,6 +582,18 @@ module.exports = () => { done(); }); }); + it('should store the user who edited the tag and return it', async () => { + const putResponse = await request(server) + .put(`/api/tags/${createdTag.id}?token=admin`) + .send({ email: 'groupa@cern.ch', mattermost: 'groupa' }); + expect(putResponse.status).to.equal(201); + + const getResponse = await request(server).get(`/api/tags/${createdTag.id}`); + expect(getResponse.status).to.equal(200); + const { data } = getResponse.body; + expect(data.lastEditedBy).to.deep.equal({ name: 'John Doe' }); + expect(data).to.not.have.property('lastEditedName'); + }); it('should return 400 if invalid email is given', (done) => { request(server) .put(`/api/tags/${createdTag.id}?token=admin`) @@ -632,6 +646,15 @@ module.exports = () => { done(); }); }); + it('should return 404 if the tag could not be found', async () => { + const response = await request(server) + .put('/api/tags/999999999?token=admin') + .send({ email: 'groupa@cern.ch', mattermost: 'groupa' }); + + expect(response.status).to.equal(404); + expect(response.body.errors[0].detail).to.equal('Tag with this id (999999999) could not be found'); + }); + it('should successfully archive the given tag', async () => { const now = Date.now(); const response = await request(server).put(`/api/tags/${createdTag.id}?token=admin`).send({ archivedAt: now }); diff --git a/test/lib/usecases/tag/CreateTagUseCase.test.js b/test/lib/usecases/tag/CreateTagUseCase.test.js index 6a5b39bfc1..47c0f91f9e 100644 --- a/test/lib/usecases/tag/CreateTagUseCase.test.js +++ b/test/lib/usecases/tag/CreateTagUseCase.test.js @@ -15,6 +15,10 @@ const { repositories: { TagRepository } } = require('../../../../lib/database/in const { tag: { CreateTagUseCase } } = require('../../../../lib/usecases/index.js'); const { dtos: { CreateTagDto } } = require('../../../../lib/domain/index.js'); const chai = require('chai'); +const assert = require('assert'); +const { BadParameterError } = require('../../../../lib/server/errors/BadParameterError.js'); +const { NotFoundError } = require('../../../../lib/server/errors/NotFoundError.js'); +const { ConflictError } = require('../../../../lib/server/errors/ConflictError.js'); const { expect } = chai; @@ -27,6 +31,7 @@ module.exports = () => { text: `TAG#${new Date().getTime()}`, }, }); + createTagDto.session = { id: 1, externalId: 1, name: 'John Doe' }; }); it('should insert a new Tag', async () => { @@ -49,7 +54,7 @@ module.exports = () => { expect(result.text).to.equal(expectedTitle); }); - it('should return null if we are trying to create the same tag again', async () => { + it('should reject with a ConflictError if we are trying to create the same tag again', async () => { const nTagsBefore = await TagRepository.count(); await new CreateTagUseCase() @@ -58,13 +63,43 @@ module.exports = () => { const nTagsAfter = await TagRepository.count(); expect(nTagsAfter).to.be.greaterThan(nTagsBefore); - const result = await new CreateTagUseCase() - .execute(createTagDto); - - expect(result).to.be.null; + await assert.rejects( + () => new CreateTagUseCase().execute(createTagDto), + new ConflictError('The provided entity already exists'), + ); expect(await TagRepository.count()).to.equal(nTagsAfter); }); + it('should store the id of the user creating the tag', async () => { + const tag = await new CreateTagUseCase().execute(createTagDto); + expect(tag).to.not.have.property('lastEditedName'); + + const storedTag = await TagRepository.findOne({ where: { id: tag.id } }); + expect(storedTag.lastEditedByUserId).to.equal(1); + }); + + it('should reject the creation if no user is provided', async () => { + delete createTagDto.session; + const nTagsBefore = await TagRepository.count(); + + await assert.rejects( + () => new CreateTagUseCase().execute(createTagDto), + new BadParameterError('A user is required to create a tag'), + ); + expect(await TagRepository.count()).to.equal(nTagsBefore); + }); + + it('should reject the creation if the user does not exist', async () => { + createTagDto.session = { id: 9999, externalId: 9999, name: 'Ghost' }; + const nTagsBefore = await TagRepository.count(); + + await assert.rejects( + () => new CreateTagUseCase().execute(createTagDto), + new NotFoundError('User with this id (9999) could not be found'), + ); + expect(await TagRepository.count()).to.equal(nTagsBefore); + }); + it('should successfully create a new tag with a description', async () => { createTagDto.body.description = 'A description'; const tag = await new CreateTagUseCase().execute(createTagDto); diff --git a/test/lib/usecases/tag/DeleteTagUseCase.test.js b/test/lib/usecases/tag/DeleteTagUseCase.test.js index bad598f919..336b38818e 100644 --- a/test/lib/usecases/tag/DeleteTagUseCase.test.js +++ b/test/lib/usecases/tag/DeleteTagUseCase.test.js @@ -28,6 +28,7 @@ module.exports = () => { }, }); + createTagDto.session = { id: 1, externalId: 1, name: 'John Doe' }; createdTag = await new CreateTagUseCase() .execute(createTagDto); }); diff --git a/test/lib/usecases/tag/GetAllTagsUseCase.test.js b/test/lib/usecases/tag/GetAllTagsUseCase.test.js index 4f58839cfe..a9c5882c24 100644 --- a/test/lib/usecases/tag/GetAllTagsUseCase.test.js +++ b/test/lib/usecases/tag/GetAllTagsUseCase.test.js @@ -28,6 +28,19 @@ module.exports = () => { expect(tags).to.be.an('array'); }); + + it('should return the user who last edited each tag', async () => { + const { tags } = await new GetAllTagsUseCase().execute(); + + for (const tag of tags) { + expect(tag).to.have.property('lastEditedBy'); + expect(tag).to.not.have.property('lastEditedName'); + expect(tag).to.not.have.property('lastEditedByUserId'); + if (tag.lastEditedBy !== null) { + expect(tag.lastEditedBy).to.have.all.keys('name'); + } + } + }); it('should return tags sorted by text', async () => { const { tags } = await new GetAllTagsUseCase().execute(); diff --git a/test/lib/usecases/tag/GetTagUseCase.test.js b/test/lib/usecases/tag/GetTagUseCase.test.js index 4395c47afb..b8b94e98b0 100644 --- a/test/lib/usecases/tag/GetTagUseCase.test.js +++ b/test/lib/usecases/tag/GetTagUseCase.test.js @@ -12,6 +12,7 @@ */ const { tag: { GetTagUseCase } } = require('../../../../lib/usecases/index.js'); +const { repositories: { TagRepository }, utilities: { QueryBuilder } } = require('../../../../lib/database/index.js'); const { dtos: { GetTagDto } } = require('../../../../lib/domain/index.js'); const chai = require('chai'); @@ -35,4 +36,22 @@ module.exports = () => { expect(result).to.have.ownProperty('id'); expect(result.id).to.equal(1); }); + + it('should return the user who last edited the tag', async () => { + const createdTag = await TagRepository.insert({ text: `TAG-LAST-EDITED-${Date.now()}`, lastEditedByUserId: 2 }); + const result = await new GetTagUseCase().execute({ params: { tagId: createdTag.id } }); + + expect(result.lastEditedBy).to.deep.equal({ name: 'Jan Jansen' }); + + await TagRepository.removeAll(new QueryBuilder().where('id').is(createdTag.id)); + }); + + it('should return null last editor for a tag that was never edited', async () => { + const createdTag = await TagRepository.insert({ text: `TAG-NEVER-EDITED-${Date.now()}` }); + const result = await new GetTagUseCase().execute({ params: { tagId: createdTag.id } }); + + expect(result.lastEditedBy).to.be.null; + + await TagRepository.removeAll(new QueryBuilder().where('id').is(createdTag.id)); + }); }; diff --git a/test/lib/usecases/tag/UpdateTagUseCase.test.js b/test/lib/usecases/tag/UpdateTagUseCase.test.js index 959c830213..0ef90e6267 100644 --- a/test/lib/usecases/tag/UpdateTagUseCase.test.js +++ b/test/lib/usecases/tag/UpdateTagUseCase.test.js @@ -14,6 +14,9 @@ const { tag: { UpdateTagUseCase } } = require('../../../../lib/usecases/index.js'); const { dtos: { UpdateTagDto } } = require('../../../../lib/domain/index.js'); const chai = require('chai'); +const assert = require('assert'); +const { BadParameterError } = require('../../../../lib/server/errors/BadParameterError.js'); +const { NotFoundError } = require('../../../../lib/server/errors/NotFoundError.js'); const { expect } = chai; @@ -39,33 +42,48 @@ module.exports = () => { }; }); it('should save the correct values', async () => { - const { result } = await new UpdateTagUseCase() + const result = await new UpdateTagUseCase() .execute(updateTagDto); expect(result.mattermost).to.equal('tag,tag,tag'); - expect(result.lastEditedName).to.equal('John Doe'); + expect(result.lastEditedBy).to.deep.equal({ name: 'John Doe' }); + expect(result).to.not.have.property('lastEditedName'); expect(result.email).to.equal('cern@tag.ch,cern@othertag.ch'); expect(result.description).to.equal('The new tag\'s description'); expect(result.archived).to.be.true; }); - it('should return an error when values do not match', async () => { - const newTagDto = await UpdateTagDto.validateAsync({ - body: { - mattermost: 'tag,tag,tag', - email: 'cern@tag.ch', - }, - params: { - tagId: 9999, - }, - }); - newTagDto.session = { - personid: 1, - id: 1, - name: 'John Do', + it('should store the id of the user performing the update', async () => { + updateTagDto.session = { + personid: 456, + id: 2, + name: 'Jan Jansen', }; - const { error } = await new UpdateTagUseCase() - .execute(newTagDto); - expect(error.status).to.equal('400'); - expect(error.title).to.equal('this tag with this tag id: (9999) could not be found.'); + const result = await new UpdateTagUseCase() + .execute(updateTagDto); + expect(result.lastEditedBy).to.deep.equal({ name: 'Jan Jansen' }); + }); + + it('should reject when no user is provided in the session', async () => { + delete updateTagDto.session; + await assert.rejects( + () => new UpdateTagUseCase().execute(updateTagDto), + new BadParameterError('A user is required to update a tag'), + ); + }); + + it('should reject when the session user does not exist', async () => { + updateTagDto.session = { id: 9999, externalId: 9999, name: 'Ghost' }; + await assert.rejects( + () => new UpdateTagUseCase().execute(updateTagDto), + new NotFoundError('User with this id (9999) could not be found'), + ); + }); + + it('should reject when the tag does not exist', async () => { + updateTagDto.params.tagId = 9999; + await assert.rejects( + () => new UpdateTagUseCase().execute(updateTagDto), + new NotFoundError('Tag with this id (9999) could not be found'), + ); }); }; diff --git a/test/public/tags/create.test.js b/test/public/tags/create.test.js index b7a573e136..c29ac477f8 100644 --- a/test/public/tags/create.test.js +++ b/test/public/tags/create.test.js @@ -70,7 +70,7 @@ module.exports = () => { await pressElement(page, 'button#submit'); // Because this tag already exists, we expect an error message to appear - await expectInnerText(page, '.alert', 'Conflict: The provided entity already exists'); + await expectInnerText(page, '.alert', 'The request conflicts with existing data: The provided entity already exists'); }); it('Should show no fields when having no admin roles', async () => { diff --git a/test/public/tags/detail.test.js b/test/public/tags/detail.test.js index 716e324fa7..fbe34c9c79 100644 --- a/test/public/tags/detail.test.js +++ b/test/public/tags/detail.test.js @@ -12,8 +12,9 @@ */ const chai = require('chai'); -const { defaultBefore, defaultAfter, expectInnerText, pressElement, getFirstRow, goToPage, waitForNavigation } = require('../defaults.js'); +const { defaultBefore, defaultAfter, expectInnerText, expectInnerTextTo, pressElement, getFirstRow, goToPage, waitForNavigation } = require('../defaults.js'); const { resetDatabaseContent } = require('../../utilities/resetDatabaseContent.js'); +const { repositories: { TagRepository } } = require('../../../lib/database/index.js'); const { expect } = chai; @@ -60,6 +61,17 @@ module.exports = () => { expect(await emails[0].evaluate((element) => element.innerText)).to.equal('food-group@cern.ch'); }); + it('should display the name of the user who last edited the tag', async () => { + await goToPage(page, 'tag-detail', { queryParameters: { id: 1, panel: 'logs' } }); + await expectInnerTextTo(page, '#tag-lastEditedBy', (text) => text.trim().endsWith('-')); + + await TagRepository.updateAll({ lastEditedByUserId: 1 }, { where: { id: 1 } }); + await goToPage(page, 'tag-detail', { queryParameters: { id: 1, panel: 'logs' } }); + await expectInnerTextTo(page, '#tag-lastEditedBy', (text) => text.trim().endsWith('John Doe')); + + await TagRepository.updateAll({ lastEditedByUserId: null }, { where: { id: 1 } }); + }); + it('notifies if a specified tag id is invalid', async () => { // Navigate to a tag detail view with an id that cannot exist await goToPage(page, 'tag-detail', { queryParameters: { id: 'abc' } }); diff --git a/test/public/tags/overview.test.js b/test/public/tags/overview.test.js index 6b28633b6a..266493c062 100644 --- a/test/public/tags/overview.test.js +++ b/test/public/tags/overview.test.js @@ -23,6 +23,7 @@ const { waitForNavigation, } = require('../defaults.js'); const { resetDatabaseContent } = require('../../utilities/resetDatabaseContent.js'); +const { repositories: { TagRepository } } = require('../../../lib/database/index.js'); const { expect } = chai; @@ -73,6 +74,25 @@ module.exports = () => { expect(headers[4]).to.equal('Email'); }); + it('should display the name of the user who last edited the tag', async () => { + await page.waitForSelector('tbody tr[id^="row"]'); + table = await page.$$('tbody tr'); + const editedRowId = await getFirstRow(table, page); + const editedTagId = parseInt(editedRowId.slice('row'.length), 10); + + await TagRepository.updateAll({ lastEditedByUserId: 2 }, { where: { id: editedTagId } }); + await page.reload({ waitUntil: 'networkidle0' }); + + await page.waitForSelector(`#${editedRowId}-lastEditedBy`); + expect(await page.$eval(`#${editedRowId}-lastEditedBy`, (cell) => cell.innerText)).to.equal('Jan Jansen'); + + // Tags never edited display a placeholder + const uneditedCellText = await page.$eval('tbody tr:nth-child(2) td:nth-child(2)', (cell) => cell.innerText); + expect(uneditedCellText).to.equal('-'); + + await TagRepository.updateAll({ lastEditedByUserId: null }, { where: { id: editedTagId } }); + }); + it('can navigate to a tag detail page', async () => { table = await page.$$('tr'); firstRowId = await getFirstRow(table, page);