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

Remove StudyID Requirement for Single Study in Custom Selection. #4822

Closed
wants to merge 1 commit into from
Closed

Remove StudyID Requirement for Single Study in Custom Selection. #4822

wants to merge 1 commit into from

Conversation

deveshidwivedi
Copy link

Fix cBioPortal/cbioportal#10445

Changes:
Introduced a ternary operator to decide whether to include the studyId: prefix based on the isSingleStudy property. If it is a single study, only s.sampleId is used; otherwise, the original format with studyId: prefix is maintained.

@deveshidwivedi
Copy link
Author

Hi @alisman, could you please review this?

@alisman
Copy link
Collaborator

alisman commented Jan 5, 2024

@deveshidwivedi does the selection actually work without the study id prefix? in other words when you click the select button?

@deveshidwivedi
Copy link
Author

I'm unable to open it in localhost since some time, please give me some time to check and make updates.

@alisman
Copy link
Collaborator

alisman commented Jan 8, 2024

@deveshidwivedi give us another day and the issue with localhost will be resolved. we apologize. it has to do with a big release we are doing (6.0)

@alisman
Copy link
Collaborator

alisman commented Jan 8, 2024

@deveshidwivedi can you do me a favor and rebase on master branch.

@deveshidwivedi
Copy link
Author

hi @alisman, I'm still facing issues

Copy link

netlify bot commented Jan 17, 2024

Deploy Preview for cbioportalfrontend ready!

Name Link
🔨 Latest commit 8fc733f
🔍 Latest deploy log https://app.netlify.com/sites/cbioportalfrontend/deploys/65a817b8315725000815bada
😎 Deploy Preview https://deploy-preview-4822--cbioportalfrontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@alisman
Copy link
Collaborator

alisman commented Jan 17, 2024 via email

@alisman
Copy link
Collaborator

alisman commented Jan 17, 2024

@deveshidwivedi @ino based on my testing, this ALREADY works in custom selection. It doesn't work in the custom data feature, but that isn't addressed by this PR:

image

@deveshidwivedi
Copy link
Author

@alisman my bad, I'll update the code for custom data to resolve this issue. Also, the issue with local server still remains and it might be due to my machine, I am unable to see any updates for the changes I make. I'll close this pr for now and open one with an updated working code. Thanks!

@deveshidwivedi deveshidwivedi closed this by deleting the head repository Jan 18, 2024
@alisman
Copy link
Collaborator

alisman commented Jan 18, 2024

Hi Davesh,
I'm happy to do a zoom with you and try to resolve the problem. It would actually be good to do so on behalf of other contributors.

@deveshidwivedi
Copy link
Author

Thank you very much! Since i had a problem with the repo on my machine (which is windows), i tried forking it again and wasn't able to do it. Went through tutorials on the internet and finally got it on my device. I'll try to make updates again and get back here! This time it should work..

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.

Make custom data work without study_id if one study
2 participants