ImagingDataCommons / IDC-WebApp

Web Application front end for IDC (CORE REPO)
Apache License 2.0
6 stars 2 forks source link

Selection of analysis results collections in collections table leads to unexpected result #740

Closed fedorov closed 3 years ago

fedorov commented 3 years ago

If I click on a collection that is an analysis collection in the collections page, I am brought to the explorer page with URL populated with a filter, but filter blank.

Reated to this we should probably disable the ability to click on the analysis collections in the collections list until #590 is addressed.

s-paquette commented 3 years ago

@fedorov They're actually intended to be disabled, not sure why they aren't. I'll see if I can sneak in this fix for production.

s-paquette commented 3 years ago

So the issue here has to do with how we were determining the collections to select when an analysis result was clicked on. I've put in a fix for now, but I'm going to make a ticket about how this should function longer-term.

fedorov commented 3 years ago

I think long term, once there's progress for #593, clicking analysis results collection in the collections list should result in selecting that collection as a filter in the list of analysis results collections in the portal.

s-paquette commented 3 years ago

@fedorov Precisely.

fedorov commented 3 years ago

Test does not pass for me. Sometime it results in selection of collections (presumably, those that are touched by the analysis results collection?), and sometimes it does not do anything (such as for PROSTATEx-Seg-HiRes - although the filter is populated in the URL: https://testing-portal.canceridc.dev/explore/?filters_for_load=%5B%7B%22filters%22:%5B%7B%22id%22:%22120%22,%22values%22:%5B%22spie_aapm_nci%20prostatex%20challenges%20(prostate_x_challenge)%22%5D%7D%5D%7D%5D). What is the expected behavior?

fedorov commented 3 years ago

"Clear cache and hard reload" did not help.

fedorov commented 3 years ago

I will follow up with a query how to match these items to the original collection ids.

s-paquette commented 3 years ago

Okay, found these bugs, will flag this and tag both of you once the test build is up.

fedorov commented 3 years ago

@bcli4d I think you should use something like the below to get the list of collections "touched" by an analysis results collection instead of relying on scraping the wiki:

WITH
  analyses_results_studies AS (
  SELECT
    DISTINCT(StudyInstanceUID) AS StudyInstanceUID,
    analyses.DOI,
    analyses.Title
  FROM
    `canceridc-data.idc_v4.dicom_all` AS dicom_all
  JOIN
    `canceridc-data.idc_v4.analysis_results_metadata` AS analyses
  ON
    dicom_all.Source_DOI = analyses.DOI)
SELECT
  analyses_results_studies.DOI AS analysis_collection_DOI,
  STRING_AGG(DISTINCT(analyses_results_studies.Title)) as Title,
  STRING_AGG(DISTINCT(collection_id))
FROM
  analyses_results_studies
JOIN
  `canceridc-data.idc_v4.dicom_all` AS dicom_all
ON
  dicom_all.StudyInstanceUID = analyses_results_studies.StudyInstanceUID
GROUP BY
  analysis_collection_DOI

image

bcli4d commented 3 years ago

Thanks for the SQL @fedorov. OK to wait for the next version, or does the analysis_results_metadata table get its own hotfix?

s-paquette commented 3 years ago

@pgundluru @fedorov Ready for testing.

fedorov commented 3 years ago

@bcli4d would it be possible to fix the table manually, and integrate something like the query above into ETL for the next release? It does bother me that the analysis results metadata table is not what user would expect it to contain.

@s-paquette it still does not work for me in test if I pick RIDER-LungCT-Seg.

bcli4d commented 3 years ago

I'll update the table tomorrow.

bcli4d commented 3 years ago

I'm not sure how these collection IDs are intended to be used. The collection ids returned by the above script are from dicom_all, which are "idc_webapp_collection_id"s. Is that what you need @s-paquette ? Those IDs are the "tcia_api_collections_id"s, with both spaces and dashes replaced by underscores, and lowered.

bcli4d commented 3 years ago

@s-paquette:..and should the Collections value be a single string of comma separated collection ids? or an array of strings?

bcli4d commented 3 years ago

I went ahead and update didc-dev-etl.idc_v4.analysis_results_metadata. I've not pushed that to the public dataset. I'll do that when dev version is accepted.

fedorov commented 3 years ago

You meant - when the test version is accepted?

bcli4d commented 3 years ago

When the changes to idc-dev-etl.idc_v4.analysis_results_metadata are accepted.

bcli4d commented 3 years ago

idc-dev-etl.idc_v4.analysis_results_metadata copied to canceridc-data.idc_v4.analysis_results_metadata

s-paquette commented 3 years ago

@fedorov @pgundluru Hotfix applied to production (NOT in test or dev)--check when you have a moment, please!

pgundluru commented 3 years ago

Thank you Suzanne, Please see my results below:

DICOM-LIDC-IDRI - Success DICOM-SR-Breast-Clinical - Fails with no errors in console PROSTATEx-Seg-HiRes - Fails PROSTATEx-Seg-Zones - Success QIN-LungCT-Seg - Fails RIDER-LungCT-Seg - Success

s-paquette commented 3 years ago

@pgundluru I can't reproduce a failure on PROSTATEx-Seg-HiRes - can you try it again? Found the problem and fixed the other two--try again when you get a chance, please.

pgundluru commented 3 years ago

Yes, Retested and i see that sometimes i will need to refresh once to get the filters to show up

DICOM-LIDC-IDRI - Success DICOM-SR-Breast-Clinical - Success PROSTATEx-Seg-HiRes - Success PROSTATEx-Seg-Zones - Success QIN-LungCT-Seg - Success RIDER-LungCT-Seg - Success