Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix BrAPI observation search endpoint #4635

Merged
merged 2 commits into from
Oct 13, 2023
Merged

fix BrAPI observation search endpoint #4635

merged 2 commits into from
Oct 13, 2023

Conversation

timparsons
Copy link
Contributor

Description

Observations (phenotypic data) were missing from exports when BreedBase was the backing service. The cause was that the Observations search endpoint wasn't returning any data. BreedBase was interpreting the trialDbIds sent in the search POST body as both Trial and Study DbIds. This is likely due to conflicting terminology between BrAPI and BreedBase: https://gist.github.com/mlm483/b5e1590fdedd0c3dcd116bd29ef19423.

Testing

  1. Upload an experiment with Observations (phenotypic data) to a BreedBase-backed program.
  2. Download the experiment's Observation dataset.
  3. Check that downloaded file has Observations (phenotypic data).

Alternatively:

  1. Use this query to find valid folder and program DbIds:
SELECT germplasm_uniquename, trial_id, trial_name, breeding_program_id, breeding_program_name, folder_id, folder_name, observations
FROM materialized_phenotype_jsonb_table
LEFT JOIN (
    select stock.stock_id, array_agg(db.name)::text[] as xref_sources, array_agg(dbxref.accession)::text[] as xref_ids
    from stock
             join stock_dbxref sd on stock.stock_id = sd.stock_id
             join dbxref on sd.dbxref_id = dbxref.dbxref_id
             join db on dbxref.db_id = db.db_id
    group by stock.stock_id
) xref on xref.stock_id = materialized_phenotype_jsonb_table.observationunit_stock_id
;
  1. POST to the search endpoint with the folder and program DbIds from step 1.
curl --location 'http://localhost:7080/brapi/v2/search/observations/?pageSize=1000&page=0' \
--header 'Content-Type: application/json' \
--data '{ "programDbIds": ["{programDbId}"], "trialDbIds": ["{trialDbId}"], "pageSize": 10000000 }'
  1. GET the search result endpoint with the searchResultDbId returned in step 2.
curl --location 'http://localhost:7080/brapi/v2/search/observations/{searchResultDbId}?pageSize=1000&page=0'

The benefit of this approach is that you can try adding "studyDbIds": ["{studyDbId}"], to the POST call, to ensure the endpoint works as expected for search by Study as well.

Checklist

  • Refactoring only
  • Documentation only
  • Fixture update only
  • Bug fix
    • The relevant issue has been closed.
    • Further work is required.
  • New feature
    • Relevant tests have been created and run.
    • Data was added to the fixture
      • Data was added via a patch in /t/data/fixture/patches/.
    • User-Facing Change
      • The user manual in /docs has been updated.
    • Any new Perl has been documented using perldoc.
    • Any new JavaScript has been documented using JSDoc.
    • Any new legacy JavaScript has been moved from /js to /js/source/legacy.

@MFlores2021
Copy link
Contributor

MFlores2021 commented Sep 5, 2023

Breedbase doesn't have the concept of trial as BrAPI expects. So, Breedbase uses folders as BrAPI trials and the trials as BrAPI studies. A folder can have subfolders and to have backwards compatibility I created $trial_ids. And $trial_ids suppose to return $study_ids_arrayref searchs as well.
I am wondering if the issue was uploading the study ?

Copy link
Member

@lukasmueller lukasmueller left a comment

Choose a reason for hiding this comment

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

:-)

@@ -62,7 +62,7 @@ sub search {
{
bcs_schema=>$self->bcs_schema,
data_level=>$observation_level,
trial_list=>$trial_ids,
trial_list=>$study_ids_arrayref,
Copy link
Member

Choose a reason for hiding this comment

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

The code could benefit from some comments here... what are trial_ids and study_ids in this context?

@lukasmueller lukasmueller self-requested a review September 12, 2023 19:23
@lukasmueller
Copy link
Member

I think in the lab meeting we discussed that some variables should be named differently to make the code clearer.

@timparsons
Copy link
Contributor Author

I think in the lab meeting we discussed that some variables should be named differently to make the code clearer.

This has been done

Copy link
Contributor

@darekbienkowski darekbienkowski left a comment

Choose a reason for hiding this comment

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

;-)

Copy link
Member

@lukasmueller lukasmueller left a comment

Choose a reason for hiding this comment

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

:-)

@lukasmueller lukasmueller merged commit 94af34d into master Oct 13, 2023
1 check passed
@lukasmueller lukasmueller deleted the topic/BI-1843 branch October 13, 2023 13:36
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.

4 participants