feat(discord): add admin responsible field for discord admin notifica…

…tions (#427)

* add player ban revoked admin responsible

* add player name change admin responsible

* admin responsible for player skill change

src/discord/notifications/player-ban-revoked.ts

@@ -5,12 +5,13 @@ interface PlayerBanRevokedFields {
   player: string;
   reason: string;
   playerProfileUrl: string;
+  adminResponsible: string;
 }
 
 export function playerBanRevoked(fields: PlayerBanRevokedFields): MessageEmbedOptions {
   return {
     color: Colors.PlayerBanRevoked,
-    title: 'Ban revoked',
+    title: `Ban revoked by ${fields.adminResponsible}`,
     fields: [
       {
         name: 'Player',

src/discord/notifications/player-name-changed.ts

@@ -5,12 +5,13 @@ interface PlayerNameChangedFields {
   oldName: string;
   newName: string;
   profileUrl: string;
+  adminResponsible: string;
 }
 
 export function playerNameChanged(fields: PlayerNameChangedFields): MessageEmbedOptions {
   return {
     color: Colors.PlayerNameChanged,
-    title: 'Player name changed',
+    title: `Player name changed by ${fields.adminResponsible}`,
     fields: [
       {
         name: 'Old name',

src/discord/notifications/skill-changed.ts

@@ -6,15 +6,16 @@ interface SkillChangedFields {
   oldSkill: Map<string, number>;
   newSkill: Map<string, number>;
   playerProfileUrl: string;
+  adminResponsible: string;
 }
 
 export function skillChanged(fields: SkillChangedFields): MessageEmbedOptions {
   const embed: MessageEmbedOptions = {
     color: Colors.SkillChanged,
-    title: 'Player\s skill has been updated',
+    title: `Player\'s skill has been updated by ${fields.adminResponsible}`,
     fields: [
       {
-        name: 'Player name',
+        name: 'Player',
         value: `[${fields.playerName}](${fields.playerProfileUrl})`,
       },
     ],

src/players/controllers/players.controller.spec.ts

@@ -149,8 +149,8 @@ describe('Players Controller', () => {
   describe('#updatePlayer()', () => {
     it('should update the player', async () => {
       const spy = jest.spyOn(playersService, 'updatePlayer');
-      const ret = await controller.updatePlayer('FAKE_ID', { name: 'FAKE_NEW_NAME' });
-      expect(spy).toHaveBeenCalledWith('FAKE_ID', { name: 'FAKE_NEW_NAME' });
+      const ret = await controller.updatePlayer('FAKE_ID', { name: 'FAKE_NEW_NAME' }, { id: 'FAKE_ADMIN_ID' } as any);
+      expect(spy).toHaveBeenCalledWith('FAKE_ID', { name: 'FAKE_NEW_NAME' }, 'FAKE_ADMIN_ID');
       expect(ret).toEqual(playersService.player as any);
     });
   });
@@ -210,8 +210,8 @@ describe('Players Controller', () => {
     it('should set player skill', async () => {
       const skill = { soldier: 1, medic: 2 };
       const spy = jest.spyOn(playerSkillService, 'setPlayerSkill');
-      const ret = await controller.setPlayerSkill('FAKE_ID', skill);
-      expect(spy).toHaveBeenCalledWith('FAKE_ID', new Map([['soldier', 1], ['medic', 2]]));
+      const ret = await controller.setPlayerSkill('FAKE_ID', skill, { id: 'FAKE_ADMIN_ID' } as any);
+      expect(spy).toHaveBeenCalledWith('FAKE_ID', new Map([['soldier', 1], ['medic', 2]]), 'FAKE_ADMIN_ID');
       expect(ret).toEqual(playerSkillService.skill.skill);
     });
   });
@@ -237,13 +237,14 @@ describe('Players Controller', () => {
 
     it('should add player ban', async () => {
       const spy = jest.spyOn(playerBansService, 'addPlayerBan');
-      const ret = await controller.addPlayerBan('FAKE_ID', ban as any, { id: '5d448875b963ff7e00c6b6b3' });
+      const ret = await controller.addPlayerBan('FAKE_ID', ban as any, { id: '5d448875b963ff7e00c6b6b3' } as any);
       expect(spy).toHaveBeenCalledWith(ban);
       expect(ret).toEqual(ban as any);
     });
 
     it('should fail if the authorized user id is not the same as admin\'s', async () => {
-      await expect(controller.addPlayerBan('FAKE_ID', ban as any, { id: 'SOME_ID' })).rejects.toThrow(BadRequestException);
+      await expect(controller.addPlayerBan('FAKE_ID', ban as any, { id: 'SOME_ID' } as any))
+        .rejects.toThrow(BadRequestException);
     });
   });
 });

src/players/controllers/players.controller.ts

@@ -37,8 +37,8 @@ export class PlayersController {
 
   @Patch(':id')
   @Auth('admin', 'super-user')
-  async updatePlayer(@Param('id', ObjectIdValidationPipe) playerId: string, @Body() player: Partial<Player>) {
-    return await this.playersService.updatePlayer(playerId, player);
+  async updatePlayer(@Param('id', ObjectIdValidationPipe) playerId: string, @Body() player: Partial<Player>, @User() user: Player) {
+    return await this.playersService.updatePlayer(playerId, player, user.id);
   }
 
   @Get(':id/games')
@@ -93,8 +93,12 @@ export class PlayersController {
   @Put(':id/skill')
   @Auth('admin', 'super-user')
   // todo validate skill
-  async setPlayerSkill(@Param('id', ObjectIdValidationPipe) playerId: string, @Body() newSkill: { [className: string]: number }) {
-    return (await this.playerSkillService.setPlayerSkill(playerId, new Map(Object.entries(newSkill)))).skill;
+  async setPlayerSkill(
+    @Param('id', ObjectIdValidationPipe) playerId: string,
+    @Body() newSkill: Record<string, number>,
+    @User() user: Player,
+  ) {
+    return (await this.playerSkillService.setPlayerSkill(playerId, new Map(Object.entries(newSkill)), user.id))?.skill;
   }
 
   @Get(':id/bans')
@@ -106,7 +110,7 @@ export class PlayersController {
   @Post(':id/bans')
   @Auth('admin', 'super-user')
   @UsePipes(ValidationPipe)
-  async addPlayerBan(@Param('id', ObjectIdValidationPipe) playerId: string, @Body() playerBan: PlayerBan, @User() user: any) {
+  async addPlayerBan(@Param('id', ObjectIdValidationPipe) playerId: string, @Body() playerBan: PlayerBan, @User() user: Player) {
     if (playerBan.admin.toString() !== user.id) {
       throw new BadRequestException('the admin field must be the same as authorized user\'s id');
     }
@@ -117,7 +121,7 @@ export class PlayersController {
   @Auth('admin', 'super-user')
   @HttpCode(200)
   async updatePlayerBan(@Param('playerId', ObjectIdValidationPipe) playerId: string, @Param('banId', ObjectIdValidationPipe) banId: string,
-                        @Query('revoke') revoke: any) {
+                        @Query('revoke') revoke: any, @User() user: Player) {
     const player = await this.playersService.getById(playerId);
     if (!player) {
       throw new NotFoundException('player not found');
@@ -133,7 +137,7 @@ export class PlayersController {
     }
 
     if (revoke !== undefined) {
-      return this.playerBansService.revokeBan(banId);
+      return this.playerBansService.revokeBan(banId, user.id);
     }
   }
 

src/players/services/player-bans.service.spec.ts

@@ -183,7 +183,7 @@ describe('PlayerBansService', () => {
 
   describe('#revokeBan()', () => {
     it('should revoke the ban', async () => {
-      const ban = await service.revokeBan(mockPlayerBan.id);
+      const ban = await service.revokeBan(mockPlayerBan.id, admin.id);
       expect(ban.end.getTime()).toBeLessThanOrEqual(new Date().getTime());
     });
 
@@ -193,13 +193,30 @@ describe('PlayerBansService', () => {
         done();
       });
 
-      await service.revokeBan(mockPlayerBan.id);
+      await service.revokeBan(mockPlayerBan.id, admin.id);
     });
 
     it('should send discord notification', async () => {
       const spy = jest.spyOn(discordService.getAdminsChannel(), 'send');
-      const ban = await service.revokeBan(mockPlayerBan.id);
+      const ban = await service.revokeBan(mockPlayerBan.id, admin.id);
       expect(spy).toHaveBeenCalled();
     });
+
+    describe('when attempting to revoke an already expired ban', () => {
+      beforeEach(async () => {
+        mockPlayerBan.end = new Date();
+        await mockPlayerBan.save();
+      });
+
+      it('should reject', async () => {
+        await expect(service.revokeBan(mockPlayerBan.id, admin.id)).rejects.toThrowError();
+      });
+    });
+
+    describe('when the provided admin does not exist', () => {
+      it('should reject', async () => {
+        await expect(service.revokeBan(mockPlayerBan.id, new ObjectId().toString())).rejects.toThrowError();
+      });
+    });
   });
 });

src/players/services/player-bans.service.ts

@@ -91,7 +91,12 @@ export class PlayerBansService implements OnModuleInit {
     return addedBan;
   }
 
-  async revokeBan(banId: string): Promise<DocumentType<PlayerBan>> {
+  async revokeBan(banId: string, adminId: string): Promise<DocumentType<PlayerBan>> {
+    const admin = await this.playersService.getById(adminId);
+    if (!admin) {
+      throw new Error('this admin does not exist');
+    }
+
     const ban = await this.playerBanModel.findById(banId);
 
     if (ban.end < new Date()) {
@@ -109,6 +114,7 @@ export class PlayerBansService implements OnModuleInit {
       player: player.name,
       reason: ban.reason,
       playerProfileUrl: `${this.environment.clientUrl}/player/${player.id}`,
+      adminResponsible: admin.name,
     }));
 
     return ban;

src/players/services/player-skill.service.spec.ts

@@ -193,6 +193,15 @@ describe('PlayerSkillService', () => {
         await expect(service.setPlayerSkill(new ObjectId().toString(), new Map([['scout', 1]]))).rejects.toThrowError('no such player');
       });
     });
+
+    describe('when the admin id is provided', () => {
+      describe('and the provided admin does not exist', () => {
+        it('should reject', async () => {
+          await expect(service.setPlayerSkill(mockPlayer.id, new Map([['soldier', 4]]), new ObjectId().toString()))
+            .rejects.toThrowError();
+        });
+      });
+    });
   });
 
   describe('#exportPlayerSkills()', () => {

src/players/services/player-skill.service.ts

@@ -13,6 +13,7 @@ import { Etf2lProfileService } from './etf2l-profile.service';
 import { DiscordService } from '@/discord/services/discord.service';
 import { skillChanged } from '@/discord/notifications';
 import { Environment } from '@/environment/environment';
+import { Player } from '../models/player';
 
 @Injectable()
 @Console()
@@ -46,7 +47,15 @@ export class PlayerSkillService implements OnModuleInit {
     return await this.playerSkillModel.findOne({ player: playerId });
   }
 
-  async setPlayerSkill(playerId: string, skill: Map<string, number>): Promise<DocumentType<PlayerSkill>> {
+  async setPlayerSkill(playerId: string, skill: Map<string, number>, adminId?: string): Promise<DocumentType<PlayerSkill>> {
+    let admin: DocumentType<Player>;
+    if (adminId) {
+      admin = await this.playersService.getById(adminId);
+      if (!admin) {
+        throw new Error('invalid admin');
+      }
+    }
+
     const player = await this.playersService.getById(playerId);
     if (!player) {
       throw new Error('no such player');
@@ -63,6 +72,7 @@ export class PlayerSkillService implements OnModuleInit {
           oldSkill: oldSkill.skill,
           newSkill: skill,
           playerProfileUrl: `${this.environment.clientUrl}/player/${player.id}`,
+          adminResponsible: admin?.name,
         }));
         break;
       }

src/players/services/players.service.spec.ts

@@ -14,6 +14,7 @@ import { typegooseTestingModule } from '@/utils/testing-typegoose-module';
 import { MongoMemoryServer } from 'mongodb-memory-server';
 import { SteamApiService } from './steam-api.service';
 import { DiscordService } from '@/discord/services/discord.service';
+import { ObjectId } from 'mongodb';
 
 jest.mock('@/discord/services/discord.service');
 
@@ -326,16 +327,27 @@ describe('PlayersService', () => {
   });
 
   describe('#updatePlayer()', () => {
+    let admin: DocumentType<Player>;
+
+    beforeEach(async () => {
+      admin = await playerModel.create({
+        name: 'FAKE_ADMIN_NAME',
+        steamId: 'FAKE_ADMIN_STEAM_ID',
+        etf2lProfileId: 1,
+        hasAcceptedRules: true,
+      });
+    });
+
     it('should update player name', async () => {
-      const ret = await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' });
+      const ret = await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' }, admin.id);
       expect(ret.name).toEqual('NEW_NAME');
     });
 
     it('should update player role', async () => {
-      const ret1 = await service.updatePlayer(mockPlayer.id, { role: 'admin' });
+      const ret1 = await service.updatePlayer(mockPlayer.id, { role: 'admin' }, admin.id);
       expect(ret1.role).toEqual('admin');
 
-      const ret2 = await service.updatePlayer(mockPlayer.id, { role: null });
+      const ret2 = await service.updatePlayer(mockPlayer.id, { role: null }, admin.id);
       expect(ret2.role).toBe(null);
     });
 
@@ -344,20 +356,25 @@ describe('PlayersService', () => {
       jest.spyOn(onlinePlayersService, 'getSocketsForPlayer').mockReturnValue([ socket ] as any);
       const spy = jest.spyOn(socket, 'emit');
 
-      await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' });
+      await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' }, admin.id);
       expect(spy).toHaveBeenCalledWith('profile update', { name: 'NEW_NAME' });
     });
 
     it('should return null if the given player does not exist', async () => {
-      jest.spyOn(service, 'getById').mockResolvedValue(null);
-      expect(await service.updatePlayer('FAKE_ID', { })).toBeNull();
+      expect(await service.updatePlayer(new ObjectId().toString(), { }, admin.id)).toBeNull();
     });
 
     it('should notify admins on Discord', async () => {
       const spy = jest.spyOn(discordService.getAdminsChannel(), 'send');
-      await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' });
+      await service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' }, admin.id);
       expect(spy).toHaveBeenCalled();
     });
+
+    describe('when the admin does not exist', () => {
+      it('should reject', async () => {
+        await expect(service.updatePlayer(mockPlayer.id, { name: 'NEW_NAME' }, new ObjectId().toString())).rejects.toThrowError();
+      });
+    });
   });
 
   describe('#acceptTerms', () => {

src/players/services/players.service.ts

@@ -122,7 +122,12 @@ export class PlayersService {
     return await this.playerModel.find({ twitchTvUser: { $exists: true } });
   }
 
-  async updatePlayer(playerId: string, update: Partial<Player>): Promise<DocumentType<Player>> {
+  async updatePlayer(playerId: string, update: Partial<Player>, adminId: string): Promise<DocumentType<Player>> {
+    const admin = await this.getById(adminId);
+    if (!admin)  {
+      throw new Error('admin does not exist');
+    }
+
     const player = await this.getById(playerId);
     if (player) {
       if (update.name) {
@@ -133,6 +138,7 @@ export class PlayersService {
           oldName,
           newName: player.name,
           profileUrl: `${this.environment.clientUrl}/player/${player.id}`,
+          adminResponsible: admin.name,
         }));
       }