-
Notifications
You must be signed in to change notification settings - Fork 0
Show Intermediate Rapid resolver page before redirecting to Ensembl site #14
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
Conversation
{% macro render_url(url) %} | ||
<div> | ||
<span class="search-param-label">Redirecting to </span> | ||
<a target="_blank" rel="noopener noreferrer" class="link" href="{{ url }}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target="_blank"
will open the beta site in a different tab. Shouldn't it open in the same tab?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed it. There is no need to click on it as Its redirected automatically in the same tab. Not sure whether we need to open the url in new tab when user clicks on it
app/static/css/rapid_page.css
Outdated
margin: 0; | ||
padding: 0; | ||
height: 100%; | ||
font-family: Lato, 'Helvetica Neue', Helvetica, Roboto, Arial, sans-serif; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the Lato font loaded anywhere?
<div class="topbar"> | ||
<div class="topbar-left"> | ||
<div class="home-link"> | ||
<a href="https://beta.ensembl.org/" rel="noopener noreferrer"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why rel="noopener noreferrer"
?
app/api/resources/rapid_view.py
Outdated
return resolved_response(url, request) | ||
response = ResolvedURLResponse( | ||
response_type=RapidRedirectResponseType.INFO, | ||
status_code=308, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused about the relationship between the content of the page and the http status code.
If the server responds to the requested url with an html page, then shouldn't the http status code be 200? If the server redirects to a url that is different from the requested url, then the http status code should be in the 300 range. Perhaps I am misunderstanding the code here, but is the server responding with an html page, on the same url as was requested, but combines this response with a 308 http status code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the field to code
.
Rapid resolver has 2 responses
- Json (when we pass
application/json
header) - Html
There is no direct URL redirect now, You either get Json or Html response. Redirect happens after 10 seconds of opening the rapid redirect page.
If application/json
header is not passed, Html page always renders irrespective of the error and the error message will be displayed on the webpage with the status code in the venn diagram. The status code to be displayed in venn diagram is stored in ResolvedURLResponse.
We don't return 3XX code for Json.
For Html 3XX status code will displayed in the web page but API return code will be 200.
Here is sample json request/response
curl -X 'GET' \
'https://resolver.ensembl.org/rapid/Camarhynchus_parvulus_GCA_902806625.1' \
-H 'accept: application/json'
{"resolved_url":"https://beta.ensembl.org/species/2df86696-b80f-475c-b327-590a36260c6e"}
The reason I don't return 3XX for Json response is because Swagger doesn't handle 3XX response codes. We don't actually need Json response but its needed for Swagger.
<div class="error-message"> | ||
{{ response.message }} | ||
</div> | ||
{{ url_helper.render_url("https://beta.ensembl.org/") }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To which page on the beta site will the user be redirected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to which page we need to redirect in case of error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 404, the XD suggests to redirect to Species selector. For other errors, where (by definition) there is no succesfully resolved URL, we can use 404 template (with redirect to home page or species selector), or skip redirection altogether (only showing the error message).
<link rel="preconnect" href="https://fonts.googleapis.com"> | ||
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||
<link href="https://fonts.googleapis.com/css2?family=Lato:ital,wght@0,100;0,300;0,400;0,700;0,900;1,100;1,300;1,400;1,700;1,900&display=swap" rel="stylesheet"> | ||
<link href="https://fonts.googleapis.com/css2?family=Lato:ital,wght@0,100;0,300;0,400;0,700;0,900;1,100;1,300;1,400;1,700;1,900&family=Roboto:ital,wght@0,100..900;1,100..900&display=swap" rel="stylesheet"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check if you need all those font weights and font styles. Also, why roboto? And why is Lato italic both times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have dev version of XD. I'll update it after checking the XD
For now removed italics and Roboto and kept light, regular, bold weights.
response = RapidResolverResponse( | ||
response_type=RapidResolverHtmlResponseType.INFO, | ||
code=308, | ||
resolved_url=url, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ticket also mentions the case of multiple resolved URLs (code 300), which is not addressed here. In principle an assembly accession ID can resolve to multiple genome UUIDs (if it's part of both partial and integrated release, like the human reference), but it shouldn't apply to rapid species so it's fine (assuming any rapid assembly won't become part of a partial release in the future).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Not sure if we need to change anything for error redirection, but it should work well as-is.
![]() This may be somewhat tangential to the scope of your PR; but I don't think If you visit the rapid site, you will find there two url patterns for the BLAST page:
|
The following url breaks: ![]() |
app/api/resources/rapid_view.py
Outdated
return resolved_response(response, request) | ||
|
||
|
||
@router.get("/Blast", name="Resolve rapid blast page") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably don't need this.
b317870
to
b7558cf
Compare
This PR adds a redirect webpage before redirecting to Ensembl beta site from Rapid site. It automatically redirects to Ensembl site from Intermediate page after 10 seconds.
Fixes ENSWBSITES-2952
Example redirect views (For local testing use
http://localhost:8001/rapid/
)https://resolver.ensembl.org/rapid/
https://resolver.ensembl.org/rapid/Blast
https://resolver.ensembl.org/rapid/info/index.html
https://resolver.ensembl.org/rapid/Camarhynchus_parvulus_GCA_902806625.1
https://resolver.ensembl.org/rapid/Camarhynchus_parvulus_GCA_902806625.1/Location%2FView?r=2%3A361680-384534
https://resolver.ensembl.org/rapid/Camarhynchus_parvulus_GCA_902806625.1/Gene?r=2%3A361680-384534;g=ENSCPVG00005003898
https://resolver.ensembl.org/rapid/test