search

icgc-argo / platform-api

https://api.platform.icgc-argo.org/graphql
GNU Affero General Public License v3.0
0 stars 0 forks source link

🗺️ 662 Filter Programs by Region #704

Closed demariadaniel closed 8 months ago

demariadaniel commented 8 months ago

Description of changes

Filters programs by requested regions

Type of Change

daniel-cy-lu commented 8 months ago

@joneubank @demariadaniel I have ticket #690 . Where I am to remove the "regions" field in the Program Type. I see Dan is using program.regions as part of his filter and this would be broken if I am to remove regions.

demariadaniel commented 8 months ago

@joneubank @demariadaniel I have ticket #690 . Where I am to remove the "regions" field in the Program Type. I see Dan is using program.regions as part of his filter and this would be broken if I am to remove regions.

Thanks for mentioning, I saw that too, makes things confusing

daniel-cy-lu commented 8 months ago

@demariadaniel @joneubank Jon was at my desk looking at this PR. So we won't be using regions going forward. Instead we will be using dataCenter field. So maybe we do the same thing filtering by dataCenter. I have a ticket #693 that I will be adding dataCenter to the Program type. It had a blocker that just got resolved. I will work on it now, so Dan you can modify your filter.

demariadaniel commented 8 months ago

@demariadaniel @joneubank Jon was at my desk looking at this PR. So we won't be using regions going forward. Instead we will be using dataCenter field. So maybe we do the same thing filtering by dataCenter. I have a ticket #693 that I will be adding dataCenter to the Program type. It had a blocker that just got resolved. I will work on it now, so Dan you can modify your filter.

Thank you! I will revert this to a draft PR for now. I will follow your changes!

daniel-cy-lu commented 8 months ago

@demariadaniel dataCenter is ready. It's a property in the gql Program type.

demariadaniel commented 8 months ago

It would be better to modify the existing programs query to add an optional filter by datacenter ID. We've definitely added new queries like this before but you can see the amount of noise that generates.

Can we rebuild it as a filter instead of as a new query?

Yes, good call. Updated in https://github.com/icgc-argo/platform-api/pull/704/commits/821fde1e991a136dfe5724a86e666348fd41c3c5

ciaranschutte commented 8 months ago

@demariadaniel ah I thought my approval would be enough to merge. looks like you specifically need whoever requested changes approval. @joneubank