feat: pid posting option at Instrument creation

[abdellah257]

Description

• add pid to instrument creation DTO as optional field.

Motivation

• Ability to put custom pid for instruments.

Changes:

• changes to Instrument creation DTO, and schema.

Tests included

• Included for each change/fix?
• Passing?

Documentation

• swagger documentation updated (required for API changes)
• official documentation updated

official documentation info

Summary by Sourcery

Allow instruments to be created with an optional client-provided PID instead of always requiring an auto-generated one.

New Features:

• Support providing an optional custom PID when creating an instrument via the API.

Enhancements:

• Relax PID schema requirements so it is no longer mandatory and keep UUID-based default generation.
• Hide an internal instrument field from the Swagger API documentation.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In CreateInstrumentDto, consider using @ApiPropertyOptional() instead of @ApiProperty({ required: false }) for pid to better reflect its optional nature in the Swagger schema.
• Now that pid is optional in the schema, verify that the service layer consistently treats undefined vs. an empty string/null so that the default generator is only bypassed when a truly custom PID is provided.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `CreateInstrumentDto`, consider using `@ApiPropertyOptional()` instead of `@ApiProperty({ required: false })` for `pid` to better reflect its optional nature in the Swagger schema.
- Now that `pid` is optional in the schema, verify that the service layer consistently treats `undefined` vs. an empty string/null so that the default generator is only bypassed when a truly custom PID is provided.

## Individual Comments

### Comment 1
<location path="src/instruments/dto/create-instrument.dto.ts" line_range="7-14" />
<code_context>

 @ApiTags("instruments")
 export class CreateInstrumentDto extends UpdateInstrumentDto {
+  @IsOptional()
+  @ApiProperty({
+    type: String,
+    required: false,
+    uniqueItems: true,
+  })
+  @IsString()
+  readonly pid: string;
+
   @ApiProperty({
</code_context>
<issue_to_address>
**suggestion:** Consider preventing empty strings for `pid` so they don't override the auto-generated value with an effectively unusable PID.

Because `pid` is optional and only uses `@IsString()`, an empty string is currently accepted and will bypass the schema’s default generator. If blank/whitespace PIDs are not valid in your domain, add `@IsNotEmpty()` (and optionally trim the input) so only meaningful IDs are accepted when supplied.

Suggested implementation:

```typescript
import { ApiProperty, ApiTags } from "@nestjs/swagger";
import { Transform } from "class-transformer";
import { IsNotEmpty, IsOptional, IsString } from "class-validator";
import { UpdateInstrumentDto } from "./update-instrument.dto";

```

```typescript
export class CreateInstrumentDto extends UpdateInstrumentDto {
  @IsOptional()
  @ApiProperty({
    type: String,
    required: false,
    uniqueItems: true,
  })
  @Transform(({ value }) =>
    typeof value === "string" ? value.trim() : value,
  )
  @IsString()
  @IsNotEmpty()
  readonly pid: string;

```
</issue_to_address>

### Comment 2
<location path="src/instruments/dto/create-instrument.dto.ts" line_range="11" />
<code_context>
+  @ApiProperty({
+    type: String,
+    required: false,
+    uniqueItems: true,
+  })
+  @IsString()
</code_context>
<issue_to_address>
**suggestion:** `uniqueItems` is not meaningful for a scalar string property in `ApiProperty`.

`uniqueItems` in Swagger metadata only applies to array schemas, so using it on scalar `pid: string` is misleading and has no runtime effect. Please remove `uniqueItems` here and rely on the schema’s `unique: true` constraint to indicate uniqueness instead.

Suggested implementation:

```typescript
  @IsOptional()
  @ApiProperty({
    type: String,
    required: false,
  })
  @IsString()
  readonly pid: string;

```

```typescript
  @ApiProperty({
    type: String,

```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}