Skip to content

feat: add sampleName field to Sample schema#2747

Merged
Junjiequan merged 3 commits into
masterfrom
add-sampeNamefield-to-sampleSchema
May 22, 2026
Merged

feat: add sampleName field to Sample schema#2747
Junjiequan merged 3 commits into
masterfrom
add-sampeNamefield-to-sampleSchema

Conversation

@Junjiequan
Copy link
Copy Markdown
Member

@Junjiequan Junjiequan commented May 21, 2026

Description

This PR adds sampleName field to the Sample schema. Since existing samples were created without this field, a migration has been added to backfill sampleName with the existing sampleId value.

Motivation

Fixes

  • Bug fixed (#X)

Changes:

  • changes made

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

Add a required sampleName field to the Sample model and backfill existing records from sampleId.

New Features:

  • Introduce sampleName as a required field on the Sample schema and update DTOs to accept it.

Enhancements:

  • Add a one-way migration to populate sampleName for existing Sample documents using their sampleId values.

@Junjiequan Junjiequan requested a review from a team as a code owner May 21, 2026 09:59
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In UpdateSampleDto, sampleName is marked as required, which may break existing partial update flows; consider making it optional or aligning its validation with how PATCH/partial updates are intended to work for this resource.
  • The migration down method is currently a no-op; if rollback is expected in your migration flow, consider unsetting sampleName on documents where it was backfilled to avoid leaving irreversible data shape changes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `UpdateSampleDto`, `sampleName` is marked as required, which may break existing partial update flows; consider making it optional or aligning its validation with how PATCH/partial updates are intended to work for this resource.
- The migration `down` method is currently a no-op; if rollback is expected in your migration flow, consider unsetting `sampleName` on documents where it was backfilled to avoid leaving irreversible data shape changes.

## Individual Comments

### Comment 1
<location path="src/samples/dto/update-sample.dto.ts" line_range="26" />
<code_context>
+   */
+  @IsString()
+  @IsNotEmpty()
+  readonly sampleName: string;
+
   /**
</code_context>
<issue_to_address>
**question:** Consider whether `sampleName` should be optional in an update DTO

For an update DTO, requiring `sampleName` means every update must supply it, even when modifying other fields only. If this endpoint is intended for partial updates, consider marking `sampleName` as `@IsOptional()` and `readonly sampleName?: string`, and keep it required only in the create DTO. If updates are meant to be full replacements, this is fine—just confirm it aligns with the API contract.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/samples/dto/update-sample.dto.ts Outdated
@Junjiequan Junjiequan merged commit c6f6059 into master May 22, 2026
15 checks passed
@Junjiequan Junjiequan deleted the add-sampeNamefield-to-sampleSchema branch May 22, 2026 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants