Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

CRDCDH-660 Shorter Batch IDs #251

Merged
merged 7 commits into from
Jan 4, 2024
Merged

CRDCDH-660 Shorter Batch IDs #251

merged 7 commits into from
Jan 4, 2024

Conversation

Alejandro-Vega
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega commented Dec 27, 2023 •
edited
Loading

Overview

This PR aims to use the shorter Batch ID from both the listBatches API and submissionQCResults API for the tables.

Change Details (Specifics)

  • Added displayID to listBatches API
  • Added displayID to submissionQCResults API
  • Used displayID for columns in Batch and QC tables
  • Added break-all to ids on table to prevent overflows
    NOTE: Merge only after BE implementation has been merged CRDCDH-695 crdc-datahub-backend#224

Related Ticket(s)

CRDCDH-660

@Alejandro-Vega Alejandro-Vega added this to the MVP-2.1.0 (M3) milestone Dec 27, 2023
@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Jan 2, 2024
@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review January 2, 2024 15:24
@Alejandro-Vega Alejandro-Vega requested a review from amattu2 January 2, 2024 15:24
amattu2
amattu2 requested changes Jan 2, 2024
View reviewed changes
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

Did not verify locally, but the core changes look good. I added one question about the type of displayID. Otherwise LGTM.

amattu2
amattu2 approved these changes Jan 2, 2024
View reviewed changes
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

LGTM. Feel free to merge when the BE is ready.

@amattu2 amattu2 mentioned this pull request Jan 4, 2024
Merged
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Jan 4, 2024
@Alejandro-Vega Alejandro-Vega merged commit 03be7f5 into mvp-2.1.0 Jan 4, 2024
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-660 branch January 4, 2024 18:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@amattu2 amattu2 amattu2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.1.0 (MVP2-M3)
Development

Successfully merging this pull request may close these issues.
2 participants
@Alejandro-Vega @amattu2