src/campaign-news-file: Allow campaign organizer to upload files

- Changed logic to allow campaign organizer to upload files
- Added testcases to cover more cases of file uploading

apps/api/src/campaign-news-file/campaign-news-file.controller.spec.ts

@@ -4,47 +4,103 @@ import { CampaignNewsFileController } from './campaign-news-file.controller'
 import { CampaignNewsFileService } from './campaign-news-file.service'
 import { S3Service } from '../s3/s3.service'
 import { PersonService } from '../person/person.service'
-import { MockPrismaService } from '../prisma/prisma-client.mock'
+import { MockPrismaService, prismaMock } from '../prisma/prisma-client.mock'
 import { CampaignService } from '../campaign/campaign.service'
 import { VaultService } from '../vault/vault.service'
 import { KeycloakTokenParsed } from '../auth/keycloak'
 import { CampaignNewsService } from '../campaign-news/campaign-news.service'
 import { NotificationsProviderInterface } from '../notifications/providers/notifications.interface.providers'
 import { SendGridNotificationsProvider } from '../notifications/providers/notifications.sendgrid.provider'
-import { MarketingNotificationsModule } from '../notifications/notifications.module'
 import { MarketingNotificationsService } from '../notifications/notifications.service'
 import { EmailService } from '../email/email.service'
 import { TemplateService } from '../email/template.service'
+import { CampaignNewsFile, Prisma } from '@prisma/client'
 
-describe('CampaignFileController', () => {
+type PersonWithCompany = Prisma.PersonGetPayload<{
+  include: {
+    company: true
+    beneficiaries: { select: { id: true } }
+    organizer: { select: { id: true } }
+  }
+}>
+
+describe('CampaignNewsFileController', () => {
   let controller: CampaignNewsFileController
   let campaignNewsFileService: CampaignNewsFileService
+  let campaignNewsService: CampaignNewsService
   let personService: PersonService
 
-  const personIdMock = 'testPersonId'
+  const fileMock: CampaignNewsFile = {
+    id: 'fileId',
+    filename: 'filename',
+    newsId: '123',
+    role: 'background',
+    personId: '12',
+    mimetype: 'image/jpeg',
+  }
+
+  const personMock: PersonWithCompany | null = {
+    id: 'e43348aa-be33-4c12-80bf-2adfbf8736cd',
+    firstName: 'John',
+    lastName: 'Doe',
+    keycloakId: 'some-id',
+    email: '[email protected]',
+    emailConfirmed: false,
+    companyId: null,
+    phone: null,
+    picture: null,
+    createdAt: new Date('2021-10-07T13:38:11.097Z'),
+    updatedAt: new Date('2021-10-07T13:38:11.097Z'),
+    newsletter: true,
+    address: null,
+    birthday: null,
+    personalNumber: null,
+    stripeCustomerId: null,
+    profileEnabled: true,
+    company: {
+      id: '1',
+      legalPersonName: 'Admin Dev',
+      companyName: 'company-test',
+      createdAt: new Date(),
+      companyNumber: '123',
+      countryCode: '123',
+      personId: '12',
+      cityId: '1',
+      updatedAt: new Date(),
+    },
+    organizer: {
+      id: '1',
+    },
+    beneficiaries: [],
+  }
+
   const fileId = 'fileId'
-  const articleId = 'testArticleId'
+  const articleId = 'e43348aa-be33-4c12-80bf-2adfbf8736cd'
   const userMock = {
-    sub: 'testKeycloackId',
+    sub: 'e43348aa-be33-4c12-80bf-2adfbf8736cd',
     resource_access: { account: { roles: [] } },
     'allowed-origins': [],
   } as KeycloakTokenParsed
 
+  const adminMock = {
+    ...userMock,
+    resource_access: { account: { roles: ['account-view-supporters'] } },
+  }
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
-      controllers: [CampaignNewsFileController, MarketingNotificationsModule],
+      controllers: [CampaignNewsFileController],
       providers: [
-        {
-          provide: CampaignNewsFileService,
-          useValue: { create: jest.fn(() => fileId) },
-        },
+        CampaignNewsFileService,
         MockPrismaService,
         EmailService,
         TemplateService,
-        S3Service,
+        {
+          provide: S3Service,
+          useValue: { uploadObject: jest.fn() },
+        },
         {
           provide: PersonService,
-          useValue: { findOneByKeycloakId: jest.fn(() => ({ id: personIdMock })) },
+          useValue: { findOneByKeycloakId: jest.fn() },
         },
         ConfigService,
         {
@@ -65,6 +121,7 @@ describe('CampaignFileController', () => {
 
     controller = module.get<CampaignNewsFileController>(CampaignNewsFileController)
     campaignNewsFileService = module.get<CampaignNewsFileService>(CampaignNewsFileService)
+    campaignNewsService = module.get<CampaignNewsService>(CampaignNewsService)
     personService = module.get<PersonService>(PersonService)
   })
 
@@ -76,34 +133,124 @@ describe('CampaignFileController', () => {
     expect(controller).toBeDefined()
   })
 
-  it('should call service for create campaign file for admin user', async () => {
+  it('should allow admins to upload files', async () => {
+    const files = [
+      { mimetype: 'jpg', originalname: 'testName1', buffer: Buffer.from('') },
+      { mimetype: 'jpg', originalname: 'testName2', buffer: Buffer.from('') },
+    ] as Express.Multer.File[]
+
+    prismaMock.campaignNews.count.mockResolvedValue(0)
+
+    const personSpy = jest.spyOn(personService, 'findOneByKeycloakId').mockResolvedValue(personMock)
+    const createSpy = jest.spyOn(campaignNewsFileService, 'create')
+    const canEditSpy = jest.spyOn(campaignNewsService, 'canEditArticle')
+    prismaMock.campaignNewsFile.create.mockResolvedValue(fileMock)
+    expect(
+      await controller.create(articleId, { roles: ['background'] }, files, {
+        ...adminMock,
+      }),
+    ).toEqual([fileId, fileId])
+
+    expect(personSpy).toHaveBeenCalledWith(userMock.sub)
+    expect(canEditSpy).toHaveBeenCalledWith(articleId, adminMock)
+    expect(await campaignNewsService.canEditArticle(articleId, adminMock)).toEqual(true)
+    expect(prismaMock.campaignNews.count).toHaveBeenCalledWith({
+      where: {
+        id: articleId,
+        campaign: { organizer: { person: { keycloakId: userMock.sub } } },
+      },
+    })
+    expect(createSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('should allow campaign organizers to upload file ', async () => {
     const files = [
       { mimetype: 'jpg', originalname: 'testName1', buffer: Buffer.from('') },
       { mimetype: 'jpg', originalname: 'testName2', buffer: Buffer.from('') },
     ] as Express.Multer.File[]
 
+    prismaMock.campaignNewsFile.create.mockResolvedValue(fileMock)
+    prismaMock.campaignNews.count.mockResolvedValue(1)
+
+    const canEditSpy = jest.spyOn(campaignNewsService, 'canEditArticle')
+    const personSpy = jest.spyOn(personService, 'findOneByKeycloakId').mockResolvedValue(personMock)
+    const createSpy = jest.spyOn(campaignNewsFileService, 'create')
     expect(
       await controller.create(articleId, { roles: ['background'] }, files, {
         ...userMock,
-        ...{ resource_access: { account: { roles: ['account-view-supporters'] } } },
       }),
     ).toEqual([fileId, fileId])
 
-    expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
-    expect(campaignNewsFileService.create).toHaveBeenCalledTimes(2)
+    expect(personSpy).toHaveBeenCalledWith(userMock.sub)
+    expect(canEditSpy).toHaveBeenCalledWith(articleId, userMock)
+    expect(await campaignNewsService.canEditArticle(articleId, adminMock)).toEqual(true)
+    expect(prismaMock.campaignNews.count).toHaveBeenCalledWith({
+      where: {
+        id: articleId,
+        campaign: { organizer: { person: { keycloakId: userMock.sub } } },
+      },
+    })
+    expect(createSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('should throw an error if neither admin or organizer tries to upload a file', async () => {
+    const files = [
+      { mimetype: 'jpg', originalname: 'testName1', buffer: Buffer.from('') },
+      { mimetype: 'jpg', originalname: 'testName2', buffer: Buffer.from('') },
+    ] as Express.Multer.File[]
+
+    prismaMock.campaignNewsFile.create.mockResolvedValue(fileMock)
+    prismaMock.campaignNews.count.mockResolvedValue(0)
+
+    const canEditSpy = jest.spyOn(campaignNewsService, 'canEditArticle')
+    const personSpy = jest.spyOn(personService, 'findOneByKeycloakId').mockResolvedValue(personMock)
+    const createSpy = jest.spyOn(campaignNewsFileService, 'create')
+    await expect(
+      controller.create(articleId, { roles: ['background'] }, files, {
+        ...userMock,
+      }),
+    ).rejects.toThrowError()
+
+    expect(personSpy).toHaveBeenCalledWith(userMock.sub)
+    expect(canEditSpy).toHaveBeenCalledWith(articleId, userMock)
+    expect(await campaignNewsService.canEditArticle(articleId, userMock)).toEqual(false)
+    expect(prismaMock.campaignNews.count).toHaveBeenCalledWith({
+      where: {
+        id: articleId,
+        campaign: { organizer: { person: { keycloakId: userMock.sub } } },
+      },
+    })
+    expect(createSpy).not.toHaveBeenCalled()
+  })
+
+  it('should allow campaign organizer to delete files', async () => {
+    prismaMock.campaignNewsFile.count.mockResolvedValue(1)
+    const removeSpy = jest.spyOn(campaignNewsFileService, 'remove').mockResolvedValue(fileMock)
+    expect(await controller.remove(fileId, userMock)).toEqual(fileMock)
+    expect(removeSpy).toHaveBeenCalledWith(fileId)
+  })
+
+  it('should allow admin to delete files', async () => {
+    prismaMock.campaignNewsFile.count.mockResolvedValue(0)
+    const removeSpy = jest.spyOn(campaignNewsFileService, 'remove').mockResolvedValue(fileMock)
+    expect(await controller.remove(fileId, adminMock)).toEqual(fileMock)
+    expect(removeSpy).toHaveBeenCalledWith(fileId)
+  })
+
+  it('should throw an error if delete request is not coming from organizer or admin', async () => {
+    prismaMock.campaignNewsFile.count.mockResolvedValue(0)
+    jest.spyOn(campaignNewsFileService, 'remove')
+    await expect(controller.remove(fileId, userMock)).rejects.toThrowError()
+    expect(campaignNewsFileService.remove).not.toHaveBeenCalled()
   })
 
   it('should throw an error for missing person', async () => {
     jest.spyOn(personService, 'findOneByKeycloakId').mockResolvedValue(null)
 
     await expect(controller.create(articleId, { roles: [] }, [], userMock)).rejects.toThrowError()
-
-    expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
   })
 
   it('should throw an error for user not having access', async () => {
     await expect(controller.create(articleId, { roles: [] }, [], userMock)).rejects.toThrowError()
-
-    expect(personService.findOneByKeycloakId).toHaveBeenCalledWith(userMock.sub)
   })
 })

apps/api/src/campaign-news-file/campaign-news-file.controller.ts

@@ -10,28 +10,26 @@ import {
   NotFoundException,
   Logger,
   Body,
-  Inject,
-  forwardRef,
   ForbiddenException,
 } from '@nestjs/common'
 import { FilesInterceptor } from '@nestjs/platform-express'
 import { UseInterceptors, UploadedFiles } from '@nestjs/common'
-import { RoleMatchingMode, Roles } from 'nest-keycloak-connect'
-import { RealmViewSupporters, ViewSupporters } from '@podkrepi-bg/podkrepi-types'
 import { Public, AuthenticatedUser } from 'nest-keycloak-connect'
 import { PersonService } from '../person/person.service'
 import { FilesRoleDto } from './dto/files-role.dto'
 import { CampaignNewsFileService } from './campaign-news-file.service'
 import { KeycloakTokenParsed, isAdmin } from '../auth/keycloak'
 import { ApiTags } from '@nestjs/swagger'
 import { validateFileType } from '../common/files'
+import { CampaignNewsService } from '../campaign-news/campaign-news.service'
 
 @ApiTags('campaign-news-file')
 @Controller('campaign-news-file')
 export class CampaignNewsFileController {
   constructor(
-    private readonly campaignFileService: CampaignNewsFileService,
-    @Inject(forwardRef(() => PersonService)) private readonly personService: PersonService,
+    private readonly campaignNewsFileService: CampaignNewsFileService,
+    private readonly personService: PersonService,
+    private readonly campaignNewsService: CampaignNewsService,
   ) {}
 
   @Post(':article_id')
@@ -51,19 +49,22 @@ export class CampaignNewsFileController {
   ) {
     const keycloakId = user.sub
     const person = await this.personService.findOneByKeycloakId(keycloakId)
+
     if (!person) {
       Logger.warn('No person record with keycloak ID: ' + keycloakId)
       throw new NotFoundException('No person record with keycloak ID: ' + keycloakId)
     }
 
-    if (!isAdmin(user)) {
+    const canEditArticle = await this.campaignNewsService.canEditArticle(articleId, user)
+
+    if (!canEditArticle) {
       throw new ForbiddenException('User has no access to this operation.')
     }
 
     const filesRole = body.roles
     return await Promise.all(
       files.map((file, key) => {
-        return this.campaignFileService.create(
+        return this.campaignNewsFileService.create(
           Array.isArray(filesRole) ? filesRole[key] : filesRole,
           articleId,
           file.mimetype,
@@ -81,7 +82,7 @@ export class CampaignNewsFileController {
     @Param('id') id: string,
     @Response({ passthrough: true }) res,
   ): Promise<StreamableFile> {
-    const file = await this.campaignFileService.findOne(id)
+    const file = await this.campaignNewsFileService.findOne(id)
     res.set({
       'Content-Type': file.mimetype,
       'Content-Disposition': 'inline; filename="' + file.filename + '"',
@@ -91,12 +92,9 @@ export class CampaignNewsFileController {
   }
 
   @Delete(':id')
-  @Roles({
-    roles: [RealmViewSupporters.role, ViewSupporters.role],
-    mode: RoleMatchingMode.ANY,
-  })
-  remove(@Param('id') id: string) {
-    console.log(` called`)
-    return this.campaignFileService.remove(id)
+  async remove(@Param('id') id: string, @AuthenticatedUser() user: KeycloakTokenParsed) {
+    const canDelete = await this.campaignNewsFileService.canDeleteNewsFile(id, user)
+    if (!canDelete) throw new ForbiddenException('User has no access for this operation')
+    return this.campaignNewsFileService.remove(id)
   }
 }

apps/api/src/campaign-news-file/campaign-news-file.module.ts

@@ -1,17 +1,15 @@
 import { Module } from '@nestjs/common'
 import { CampaignNewsFileService } from './campaign-news-file.service'
 import { CampaignNewsFileController } from './campaign-news-file.controller'
-import { PrismaService } from '../prisma/prisma.service'
+
 import { S3Service } from '../s3/s3.service'
-import { PersonService } from '../person/person.service'
+import { PersonModule } from '../person/person.module'
+import { CampaignNewsModule } from '../campaign-news/campaign-news.module'
+import { PrismaService } from '../prisma/prisma.service'
 
 @Module({
   controllers: [CampaignNewsFileController],
-  providers: [
-    CampaignNewsFileService,
-    PrismaService,
-    S3Service,
-    PersonService,
-  ],
+  imports: [CampaignNewsModule, PersonModule],
+  providers: [CampaignNewsFileService, PrismaService, S3Service],
 })
 export class CampaignNewsFileModule {}

apps/api/src/campaign-news-file/campaign-news-file.service.ts

@@ -5,6 +5,7 @@ import { CampaignFile, CampaignFileRole, Person } from '@prisma/client'
 import { S3Service } from '../s3/s3.service'
 import { PrismaService } from '../prisma/prisma.service'
 import { CreateCampaignNewsFileDto } from './dto/create-campaign-news-file.dto'
+import { KeycloakTokenParsed, isAdmin } from '../auth/keycloak'
 
 @Injectable()
 export class CampaignNewsFileService {
@@ -27,9 +28,8 @@ export class CampaignNewsFileService {
       personId: person.id,
     }
     const dbFile = await this.prisma.campaignNewsFile.create({ data: file })
-
     // Use the DB primary key as the S3 key. This will make sure it is always unique.
-    await this.s3.uploadObject(
+    const test = await this.s3.uploadObject(
       this.bucketName,
       dbFile.id,
       encodeURIComponent(filename),
@@ -60,6 +60,13 @@ export class CampaignNewsFileService {
     }
   }
 
+  async canDeleteNewsFile(id: string, user: KeycloakTokenParsed): Promise<boolean> {
+    const isFileOwner = await this.prisma.campaignNewsFile.count({
+      where: { id, person: { keycloakId: user.sub } },
+    })
+    return !!isFileOwner || isAdmin(user)
+  }
+
   async remove(id: string) {
     await this.s3.deleteObject(this.bucketName, id)
     return await this.prisma.campaignNewsFile.delete({ where: { id } })