Summary view filtering bug

[priti88]

Grouping of samples in the pie chart is not case sensitive, but if we apply filter then the code does exact string match.
Example: Local Recurrence = 11 samples, If I apply the filter only 10 samples get selected because one of the sample had a typo: 'Local recurrence' (R is in lower case).

[zhx828]

@jjgao this is the issue Adam and I saw last week. Basically this is a mysql group by issue. Since the pie chart gets the category from data directly. Should we make the frontend case insensitive?

[jjgao]

Yes, please make frontend case insensitive.

[zhx828]

@onursumer this is related to backend as well. Probably I should start to take a look?

[khzhu]

@zhx828 , which study in public portal should I use to reproduce the error? Thanks!

[khzhu]

Hi @priti88 , I am looking at the issue. Could you please let me know how to reproduce the issue? For instance, which study you were looking at, the type of chart used for grouping/filtering samples? Thanks!

[zhx828]

@khzhu I'm trying to reproduce the issue. But seems the all public studies with Local Recurrence all have been fixed. But one thing you can do is making the study view filtering insensitive. For instance, http://www.cbioportal.org/study?id=lgg_tcga&filterAttributeId=ONCOTREE_CODE&filterValues=OAST works but not http://www.cbioportal.org/study?id=lgg_tcga&filterAttributeId=ONCOTREE_CODE&filterValues=oast We want to make it case insensitive.

[khzhu]

I see, thanks @zhx828 , will do some debugging.

[khzhu]

Hi @zhx828 , after some debugging, I would say it is no longer an issue. First of all, the backend queries the clinical data by clinical attributes Ids and sample Ids only. And, there isn't any filters applied to the clinical attribute values being pulled out from the database. The frontend, on the other hand, does a filter only on a user selected option from the Oncotree pie chart.

The request URL no longer has filterValues appended to the query string which also eliminates any chances the query string to be modified.

[priti88]

@khzhu I don't think we can use the public data to debug this. You might have to to create some fake data and upload the study to your local instance to verify if this works.

[priti88]

I would recommend using any tcga study and changing the values for one of the clinical field. Example: make it all lower case or upper case.

[khzhu]

@priti88 , no worries, please. I was debugging using my local develop instance with the same public dataset downloaded.

[jjgao]

@khzhu any update on this?

[khzhu]

@jjgao , I do not think this is a problem any more. Repaste my comments on April 29.

after some debugging, I would say this is no longer an issue. First of all, the backend queries the clinical data by clinical attributes Ids and sample Ids only. And, there isn't any filters applied to the clinical attribute values being pulled out from the database. The frontend, on the other hand, is filtering based on the user selection on the Oncotree pie chart. filterAttributeId=ONCOTREE_CODE&filterValues=OAST no longer being appended to the URL query string so that the user will not be able by any chance to alter the filterValues.

[jjgao]

@khzhu I don't think I got it.

So when we have both Local Recurrence and Local recurrence, is our API pull both samples out when we query for Local Recurrence, ie. case insensitive? @khzhu @zhx828

[khzhu]

@jjgao , the API will only return those Local Recurrence samples. Is that a desirable behavior? If we have both Local Recurrence and Local recurrence samples, then we would have two different sample types/sites listed on the pie chart. So, If we want to think "Local Recurrence" as the same thing as "Local recurrence", then we should do the correction/mapping before importing/loading into the database.
Could you please show me how do you submit your query, since I cannot see any places you can enter your search text other than select a sample list from the dropdown box or from a pie chart.

[jjgao]

@khzhu I think one option is to make the web api case insensitive.

For example, currently this link works: https://www.cbioportal.org/study/summary?filterAttributeId=SEX&filterValues=Male&id=msk_impact_2017

but not this: https://www.cbioportal.org/study/summary?filterAttributeId=SEX&filterValues=MALE&id=msk_impact_2017

[jjgao]

Another reason we should make filter case insensitive is b/c the API for counting (pie chart) is case insensitive.

[jjgao]

@khzhu "group by" is case insensitive :)

https://stackoverflow.com/questions/10952696/how-to-do-a-case-sensitive-group-by

[khzhu]

@jjgao , but all queries are based on clinicalAttribute IDs sent from the view not text (clinical attributes)

<select id="getClinicalAttribute" resultType="org.cbioportal.model.ClinicalAttribute">
        SELECT
        <include refid="select">
            <property name="prefix" value=""/>
        </include>
        FROM clinical_attribute_meta
        INNER JOIN cancer_study ON clinical_attribute_meta.CANCER_STUDY_ID = cancer_study.CANCER_STUDY_ID
        WHERE clinical_attribute_meta.ATTR_ID = #{clinicalAttributeId}
        AND cancer_study.CANCER_STUDY_IDENTIFIER = #{studyId}
    </select>

[jjgao]

@khzhu what is the query when you click a slice on the pie?

[khzhu]

Hi @JJ, @hongxing, finally got the bug fixed 🙂 . Can give a quick demo tomorrow at our standup.

[inodb]

There is still an issue that on the frontend that attribute values are not case insensitive. There are PRs that fixed other parts of this issue e.g. this frontend PR fixed case insensitive attribute ids: cBioPortal/cbioportal-frontend#2716 & other related backend fixes: cBioPortal/cbioportal-frontend#2716.

But this still needs to be fixed:

See MALE in URL

https://www.cbioportal.org/study/summary?filterAttributeId=SEX&filterValues=MALE&id=acc_tcga_pan_can_atlas_2018

vs

Male in URL

https://www.cbioportal.org/study/summary?filterAttributeId=SEX&filterValues=Male&id=acc_tcga_pan_can_atlas_2018