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

Frontend should consume postData written to JSP #2933

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

alisman
Copy link
Collaborator

@alisman alisman commented Dec 13, 2019

External applications may send users to portal using http post (for large queries). JSP writes this posted data to postData variable. Frontend needs to consume this data.

@alisman alisman marked this pull request as ready for review December 13, 2019 22:19
@alisman alisman merged commit 611d1df into cBioPortal:master Dec 16, 2019
@alisman alisman deleted the acceptPostData branch December 16, 2019 15:20
@alisman alisman restored the acceptPostData branch December 16, 2019 16:00
Copy link
Contributor

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

Thnx for working on this. I would love to discuss what is precisely happening in this added code. But I am off-course 100% ok with this addition. Looking at the tests though, it is not clear what is being tested here. I am looking for a test that shows that the POST method is working.

@@ -103,6 +103,8 @@ export default class ResultsViewPage extends React.Component<

handleLegacySubmission(this.urlWrapper);

handlePostedSubmission(this.urlWrapper);
Copy link
Contributor

Choose a reason for hiding this comment

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

So if I understand this correctly, only the ResultsViewPage can handle POST requests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's correct. the server writes the post data in all cases though so any page can in theory access it.


if (useExternalFrontend) {

describe('error messaging for invalid study id(s)', function(){
Copy link
Contributor

Choose a reason for hiding this comment

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

I am lost here. This file resutspage.spec.js has describe study select page which belongs to Query Page. So, what does this test actually test? Is there also a test that checks for correct working of the POST method? It is not apparent from the test titles. It seems to only deal with checking of error messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

achhh. terrible, i forgot to update these descriptions. they are totally wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i will make PR to correct these.

"genetic_profile_ids_PROFILE_COPY_NUMBER_ALTERATION":"coadread_tcga_pub_gistic"
};

postDataToUrl(url, query);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

data is posted to page here

var url = browser.getUrl();

// make sure param in query is passed to url
return _.every(query,(item,key)=>{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pvannierop here we check that every posted parameter key has made it into the url. this could only happen as a result of our code transferring the postData variable into the URL.

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.

3 participants