Skip to content

feat(website): edit page - do not include fasta description in metadata field fastaIds#5629

Merged
anna-parker merged 14 commits intoedit-page-anyafrom
fastaHeader
Dec 5, 2025
Merged

feat(website): edit page - do not include fasta description in metadata field fastaIds#5629
anna-parker merged 14 commits intoedit-page-anyafrom
fastaHeader

Conversation

@anna-parker
Copy link
Contributor

@anna-parker anna-parker commented Dec 4, 2025

resolves #5627

The fastaIds object should contain a space separated list of fasta IDs. The website was constructing fastaIds for submissions via the edit page by concatenating full fasta headers of the form >fastaId description (and removing >) - but this lead to the backend thinking the description was another fastaId.

Also adds a description to multiple fasta files in the integration tests to ensure in future such an issue cannot be missed

Screenshot

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by appropriate, automated tests.
  • Any manual testing that has been done is documented (i.e. what exactly was tested?)

🚀 Preview: https://fastaheader.loculus.org

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@anna-parker anna-parker added the preview Triggers a deployment to argocd label Dec 4, 2025
@anna-parker anna-parker changed the title feat(website): do not include fasta description in fastaId on website edit page feat(website): do not include fasta description in fastaIds on website edit page Dec 4, 2025
@anna-parker anna-parker changed the title feat(website): do not include fasta description in fastaIds on website edit page feat(website): do not include fasta description in metadata field fastaIds when generated by website edit page Dec 4, 2025
@anna-parker anna-parker changed the title feat(website): do not include fasta description in metadata field fastaIds when generated by website edit page feat(website): edit page - do not include fasta description in metadata field fastaIds Dec 4, 2025
Copy link
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

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

LGTM

fastaHeader ??= value == null ? null : key; // Ensure fastaHeader is never null if a sequence exists
if (this.editableSequenceFiles.some((seq) => seq.fastaHeader === fastaHeader)) {
toast.error(`A sequence with the fastaHeader ${fastaHeader} already exists.`);
if (this.editableSequenceFiles.some((seq) => getFastaId(seq.fastaHeader) === getFastaId(fastaHeader))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maybe just save only the ID straightaway when loading the sequence into EditableSequence? Then we don't need to do all these conversions later on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im not sure if this conversion is complicated enough to store the fastaId as well - but Id be up for adding it if you think it makes the code cleaner!

@anna-parker
Copy link
Contributor Author

@fengelniederhammer I rerequested review as I feel like I have now changed a substantial part of the PR - lol

anna-parker and others added 2 commits December 5, 2025 14:07
Co-authored-by: Fabian Engelniederhammer <92720311+fengelniederhammer@users.noreply.github.com>
@anna-parker anna-parker merged commit 3c77286 into edit-page-anya Dec 5, 2025
38 of 39 checks passed
@anna-parker anna-parker deleted the fastaHeader branch December 5, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Triggers a deployment to argocd

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants