-
Couldn't load subscription status.
- Fork 79
adding qiimp via iframes #2582
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
adding qiimp via iframes #2582
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| # ----------------------------------------------------------------------------- | ||
|
|
||
| from tornado.web import RequestHandler | ||
|
|
||
| from qiita_db.logger import LogEntry | ||
| from qiita_db.user import User | ||
| from qiita_pet.util import convert_text_html | ||
|
|
@@ -78,6 +79,16 @@ def get(self): | |
| self.render("index.html", message=msg, level=lvl) | ||
|
|
||
|
|
||
| class IFrame(BaseHandler): | ||
| '''Open one of the IFrame pages''' | ||
| def get(self): | ||
| msg = self.get_argument('message', '') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The msg and lvl arguments aren't being used anywhere in the template, should these be removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are used in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, sounds good. |
||
| msg = convert_text_html(msg) | ||
| lvl = self.get_argument('level', '') | ||
| iframe = self.get_argument('iframe', '') | ||
| self.render("iframe.html", iframe=iframe, message=msg, level=lvl) | ||
|
|
||
|
|
||
| class MockupHandler(BaseHandler): | ||
| def get(self): | ||
| self.render("mockup.html") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| {% extends sitebase.html %} | ||
| {% block content %} | ||
|
|
||
| {% if iframe == 'qiita-terms' %} | ||
| <iframe style="margin: 0; padding: 0; width: 100%; height: 400px;" src="{% raw qiita_config.portal_dir %}/static/qiita_data_terms_of_use.html"></iframe> | ||
| {% elif iframe == 'qiimp' %} | ||
| <iframe style="margin: 0; padding: 0; width: 100%; height: 400px;" src="{{qiita_config.iframe_qiimp}}"></iframe> | ||
| {% else %} | ||
| <b>No content</b> | ||
| {% end %} | ||
|
|
||
| {% end %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ | |
|
|
||
| from qiita_core.qiita_settings import qiita_config | ||
| from qiita_core.util import is_test_environment | ||
| from qiita_pet.handlers.base_handlers import (MainHandler, NoPageHandler) | ||
| from qiita_pet.handlers.base_handlers import ( | ||
| MainHandler, NoPageHandler, IFrame) | ||
| from qiita_pet.handlers.auth_handlers import ( | ||
| AuthCreateHandler, AuthLoginHandler, AuthLogoutHandler, AuthVerifyHandler) | ||
| from qiita_pet.handlers.user_handlers import ( | ||
|
|
@@ -178,6 +179,7 @@ def __init__(self): | |
| (r"/release/download/(.*)", DownloadRelease), | ||
| (r"/vamps/(.*)", VAMPSHandler), | ||
| (r"/redbiom/(.*)", RedbiomPublicSearch), | ||
| (r"/iframe/", IFrame), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it depends on how we want to handle the parameters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see, I misunderstood this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to ensure I understand: in deploy, QIIMP will be accessed with https, but for testing (config_test.cfg, test_configuration_manager.py) it should be accessed with http? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question! In the tests we are not actually testing that QIIMP is installed or running, we are just testing that the iframes are created as expected based on the server. IMOO this is out of the scope of Qiita and the way that redbiom also works: if the service is down, Qiita simply displays an error. However, I see the source of the confusion as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AmandaBirmingham, could you let us know if this looks OK or if we need some changes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd be happy if we could just add a comment, at least in config_test.cfg, indicating that in the real world QIIMP will need to be accessed with https. I often look at tests to see examples of how I should use a tool, so a comment to explicitly point out where the test needs to do something that won't work in production would help people like me keep ourselves out of trouble :) I have no other comments/requests on this PR besides that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! Adding ... |
||
| # Plugin handlers - the order matters here so do not change | ||
| # qiita_db/jobs/(.*) should go after any of the | ||
| # qiita_db/jobs/(.*)/XXXX because otherwise it will match the | ||
|
|
||
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.
Can you add a test for this handler?