Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/v1/air/air.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { Test, TestingModule } from '@nestjs/testing';
import { AirController } from './air.controller';
import { AirService } from './air.service';
import { CreateAirAnnotationDto } from './dto/create-air-annotation.dto';
import { UpdateAirAnnotationDto } from './dto/update-air-annotation.dto';

describe('AirController', () => {
let controller: AirController;
const mockAirService = {
createNote: jest.fn(),
updateNote: jest.fn(),
};

beforeEach(async () => {
Expand Down Expand Up @@ -78,4 +80,45 @@ describe('AirController', () => {
expect(result).toMatchObject(payload);
});
});

describe('updateNote', () => {
it('passes the note id, body, and authenticated user to the service', async () => {
const dto = {
include_in_report: false,
note: 'corrected reading',
title: 'Amended review',
} as UpdateAirAnnotationDto;
const user = {
sub: 'user-123',
email: 'user@example.com',
isStaff: false,
};
const expected = { ...dto, dev_eui: 'ABC123', id: 7 };

mockAirService.updateNote.mockResolvedValue(expected);

await expect(controller.updateNote(7, dto, user)).resolves.toEqual(
expected,
);
expect(mockAirService.updateNote).toHaveBeenCalledWith(7, dto, user);
});

it('accepts a partial payload under the global validation pipe settings', async () => {
const pipe = new ValidationPipe({
whitelist: true,
forbidNonWhitelisted: true,
transform: true,
});
const payload = { note: 'corrected reading' };

const result: unknown = await pipe.transform(payload, {
type: 'body',
metatype: UpdateAirAnnotationDto,
data: '',
});

expect(result).toBeInstanceOf(UpdateAirAnnotationDto);
expect(result).toMatchObject(payload);
});
});
});
11 changes: 11 additions & 0 deletions src/v1/air/air.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
BadRequestException,
Controller,
Get,
Patch,
Post,
Body,
Param,
Expand All @@ -12,6 +13,7 @@ import {
import { AirService } from './air.service';
import { AirDataDto } from './dto/air-data.dto';
import { CreateAirAnnotationDto } from './dto/create-air-annotation.dto';
import { UpdateAirAnnotationDto } from './dto/update-air-annotation.dto';
import { JwtAuthGuard } from '../auth/guards/jwt.auth.guard';
import { parseTimeseriesRange } from '../common/timeseries-range.helper';
import {
Expand Down Expand Up @@ -53,6 +55,15 @@ export class AirController {
return this.airService.findAllNotes(devEui, month, year, user);
}

@Patch('notes/:note_id')
async updateNote(
@Param('note_id') noteId: number,
@Body() updateAirNoteDto: UpdateAirAnnotationDto,
@CurrentUser() user: AuthenticatedUser,
) {
return this.airService.updateNote(noteId, updateAirNoteDto, user);
}

@Delete('notes/:note_id')
async deleteNote(
@Param('note_id') noteId: number,
Expand Down
121 changes: 121 additions & 0 deletions src/v1/air/air.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BadRequestException } from '@nestjs/common';
import { AirService } from './air.service';
import { CreateAirAnnotationDto } from './dto/create-air-annotation.dto';
import { UpdateAirAnnotationDto } from './dto/update-air-annotation.dto';
import { SupabaseService } from '../../supabase/supabase.service';
import { TimezoneFormatterService } from '../common/timezone-formatter.service';

Expand Down Expand Up @@ -225,4 +226,124 @@ describe('AirService', () => {
expect(insertBuilder.insert).not.toHaveBeenCalled();
});
});

describe('updateNote', () => {
const existingNote = {
created_at: '2026-03-13T14:30:01.232544+00:00',
created_by: 'owner@example.com',
dev_eui: '2CF7F1C073800102',
id: 7,
include_in_report: true,
note: 'original note',
title: 'Original title',
};

function createUpdateBuilder(fetchResponse: {
data: typeof existingNote | null;
error: { message: string } | null;
}) {
const single = jest.fn();
const updateSelect = jest.fn().mockReturnValue({ single });
const updateEq = jest.fn().mockReturnValue({ select: updateSelect });
const update = jest.fn().mockReturnValue({ eq: updateEq });
const maybeSingle = jest.fn().mockResolvedValue(fetchResponse);
const fetchEq = jest.fn().mockReturnValue({ maybeSingle });
const select = jest.fn().mockReturnValue({ eq: fetchEq });

return { fetchEq, maybeSingle, select, single, update, updateEq };
}

it('updates only whitelisted fields, ignoring dev_eui/created_at in the body', async () => {
const user = { email: 'user@example.com', isStaff: false, sub: 'user-1' };
const builder = createUpdateBuilder({ data: existingNote, error: null });
const updatedNote = {
...existingNote,
include_in_report: false,
note: 'corrected note',
title: 'New title',
};
builder.single.mockResolvedValue({ data: updatedNote, error: null });
client.from.mockReturnValue(builder);

const dto = {
created_at: '2020-01-01T00:00:00Z',
dev_eui: 'FFFFFFFFFFFFFFFF',
include_in_report: false,
note: 'corrected note',
title: 'New title',
} as UpdateAirAnnotationDto;

await expect(service.updateNote(7, dto, user)).resolves.toEqual(
updatedNote,
);

// Access is asserted against the STORED dev_eui, not the body's.
expect(
(service as unknown as { assertDeviceAccess: jest.Mock })
.assertDeviceAccess,
).toHaveBeenCalledWith('2CF7F1C073800102', user);
// The update payload must never contain dev_eui or created_at.
expect(builder.update).toHaveBeenCalledWith({
include_in_report: false,
note: 'corrected note',
title: 'New title',
});
expect(builder.updateEq).toHaveBeenCalledWith('id', 7);
});

it('rejects when the note does not exist', async () => {
const builder = createUpdateBuilder({ data: null, error: null });
client.from.mockReturnValue(builder);

await expect(
service.updateNote(
999,
{ title: 'x' },
{
sub: 'user-1',
},
),
).rejects.toThrow(new BadRequestException('Air annotation not found'));
expect(builder.update).not.toHaveBeenCalled();
});

it('rejects an update with no editable fields', async () => {
const builder = createUpdateBuilder({ data: existingNote, error: null });
client.from.mockReturnValue(builder);

await expect(
service.updateNote(
7,
{ dev_eui: 'FFFFFFFFFFFFFFFF' },
{ sub: 'user-1' },
),
).rejects.toThrow(
new BadRequestException(
'At least one of title, note, or include_in_report is required',
),
);
expect(builder.update).not.toHaveBeenCalled();
});

it('propagates access denial before updating', async () => {
const builder = createUpdateBuilder({ data: existingNote, error: null });
client.from.mockReturnValue(builder);
(
service as unknown as { assertDeviceAccess: jest.Mock }
).assertDeviceAccess.mockRejectedValue(
new BadRequestException('Device not found'),
);

await expect(
service.updateNote(
7,
{ title: 'x' },
{
sub: 'intruder',
},
),
).rejects.toThrow('Device not found');
expect(builder.update).not.toHaveBeenCalled();
});
});
});
59 changes: 59 additions & 0 deletions src/v1/air/air.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { SupabaseService } from '../../supabase/supabase.service';
import { TimezoneFormatterService } from '../common/timezone-formatter.service';
import { BaseDataService } from '../common/base-data.service';
import { CreateAirAnnotationDto } from './dto/create-air-annotation.dto';
import { UpdateAirAnnotationDto } from './dto/update-air-annotation.dto';
import type { TableRow } from '../types/supabase';
import type { AuthenticatedUser } from '../auth/authenticated-user';

Expand Down Expand Up @@ -102,6 +103,64 @@ export class AirService extends BaseDataService<'cw_air_data'> {
return data;
}

async updateNote(
noteId: number,
updateAirNoteDto: UpdateAirAnnotationDto,
user: AuthenticatedUser,
) {
const client = this.supabaseService.getClient();
const { data: existingNote, error: fetchError } = (await client
.from('cw_air_annotations')
.select('*')
.eq('id', noteId)
.maybeSingle()) as QueryResult<AirAnnotationRow>;

if (fetchError) {
throw new InternalServerErrorException('Failed to fetch air annotation');
}

if (!existingNote) {
throw new BadRequestException('Air annotation not found');
}

await this.assertDeviceAccess(existingNote.dev_eui, user);

// Whitelist: UpdateAirAnnotationDto (PartialType of the create DTO) also
// admits dev_eui/created_at — never let an update re-point a note at a
// different device or reading.
const updates: Partial<
Pick<AirAnnotationRow, 'title' | 'note' | 'include_in_report'>
> = {};
if (updateAirNoteDto.title !== undefined) {
updates.title = updateAirNoteDto.title;
}
if (updateAirNoteDto.note !== undefined) {
updates.note = updateAirNoteDto.note;
}
if (updateAirNoteDto.include_in_report !== undefined) {
updates.include_in_report = updateAirNoteDto.include_in_report;
}

if (Object.keys(updates).length === 0) {
throw new BadRequestException(
'At least one of title, note, or include_in_report is required',
);
}

const { data, error: updateError } = (await client
.from('cw_air_annotations')
.update(updates)
.eq('id', noteId)
.select('*')
.single()) as QueryResult<AirAnnotationRow>;

if (updateError) {
throw new BadRequestException('Failed to update air annotation');
}

return data;
}

async deleteNote(noteId: number, user: AuthenticatedUser) {
const client = this.supabaseService.getClient();
const { data: existingNote, error: fetchError } = (await client
Expand Down
44 changes: 44 additions & 0 deletions src/v1/common/__snapshots__/v1-route-input-contract.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,28 @@ exports[`V1 Route Input Contracts matches the full v1 request contract snapshot
],
"type": "object",
},
"UpdateAirAnnotationDto": {
"properties": {
"created_at": {
"format": "date-time",
"type": "string",
},
"dev_eui": {
"type": "string",
},
"include_in_report": {
"type": "boolean",
},
"note": {
"nullable": true,
"type": "string",
},
"title": {
"type": "string",
},
},
"type": "object",
},
"UpdateDeviceNameGroupLocalDto": {
"properties": {
"group": {
Expand Down Expand Up @@ -579,6 +601,28 @@ exports[`V1 Route Input Contracts matches the full v1 request contract snapshot
},
],
},
"patch": {
"parameters": [
{
"in": "path",
"name": "note_id",
"required": true,
"schema": {
"type": "number",
},
},
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/UpdateAirAnnotationDto",
},
},
},
"required": true,
},
},
},
"/v1/air/{dev_eui}": {
"get": {
Expand Down
9 changes: 8 additions & 1 deletion src/v1/devices/devices.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@
let offlineCount = 0;

devices.forEach((device) => {
const lastUpdated = new Date(device.last_data_updated_at);

Check warning on line 234 in src/v1/devices/devices.service.ts

View workflow job for this annotation

GitHub Actions / build

Unsafe argument of type `any` assigned to a parameter of type `string | number | Date`

Check warning on line 234 in src/v1/devices/devices.service.ts

View workflow job for this annotation

GitHub Actions / build

Unsafe argument of type `any` assigned to a parameter of type `string | number | Date`
const minutesSinceLastUpdate =
(now.getTime() - lastUpdated.getTime()) / (1000 * 60);
const deviceType = Array.isArray(device.cw_device_type)
Expand Down Expand Up @@ -498,9 +498,16 @@
const startDate = new Date(start).toISOString();
const endDate = new Date(end).toISOString();

// Air rows carry their annotations/alerts, same as findData: the report
// note-editing page reads notes from this endpoint's rows.
const withAnnotations =
deviceType.data_table_v2 === 'cw_air_data'
? '*, cw_air_annotations(*), cw_air_alerts(*)'
: '*';

const { data: latestData, error: dataError } = (await client
.from(deviceType.data_table_v2)
.select('*')
.select(withAnnotations)
.eq('dev_eui', normalizedDevEui)
.gte('created_at', startDate)
.lte('created_at', endDate)
Expand Down
Loading
Loading