[GitHub] mistercrunch commented on a change in pull request #4463: New Landing Page v1.0

2018-02-23 Thread GitBox 
mistercrunch commented on a change in pull request #4463: New Landing Page v1.0
URL: 
https://github.com/apache/incubator-superset/pull/4463#discussion_r170332410
 
 

 ##
 File path: superset/views/utils.py
 ##
 @@ -0,0 +1,53 @@
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from collections import defaultdict
+
+from flask import g
+from flask_appbuilder.security.sqla import models as ab_models
+
+from superset import db
+
+
+def bootstrap_user_data(given_username=None):
 
 Review comment:
   `given_username` is confusing to me as it sounds like "Given Name" which 
means something else. It would be ok to receive `username` and to go `username 
= username or g.user.username` on the first line


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mistercrunch commented on a change in pull request #4463: New Landing Page v1.0

2018-02-23 Thread GitBox 
mistercrunch commented on a change in pull request #4463: New Landing Page v1.0
URL: 
https://github.com/apache/incubator-superset/pull/4463#discussion_r170333861
 
 

 ##
 File path: superset/views/utils.py
 ##
 @@ -0,0 +1,53 @@
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from collections import defaultdict
+
+from flask import g
+from flask_appbuilder.security.sqla import models as ab_models
+
+from superset import db
+
+
+def bootstrap_user_data(given_username=None):
+if given_username:
+username = given_username
+else:
+username = g.user.username
+
+user = (
+db.session.query(ab_models.User)
+.filter_by(username=username)
+.one()
+)
+roles = {}
+permissions = defaultdict(set)
+for role in user.roles:
 
 Review comment:
   For the welcome page you don't need all that in the payload, maybe this 
utility function should have a `include_perms=False` arg. Man I wish we had 
GraphQL instead of having to change the backend logic...


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mistercrunch commented on a change in pull request #4463: New Landing Page v1.0

2018-02-23 Thread GitBox 
mistercrunch commented on a change in pull request #4463: New Landing Page v1.0
URL: 
https://github.com/apache/incubator-superset/pull/4463#discussion_r17023
 
 

 ##
 File path: superset/views/__init__.py
 ##
 @@ -2,3 +2,4 @@
 from . import core  # noqa
 from . import sql_lab  # noqa
 from . import annotations # noqa
+from . import utils # noqa
 
 Review comment:
   this necessary? shouldn't be since no views should be registered here


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mistercrunch commented on a change in pull request #4463: New Landing Page v1.0

2018-02-23 Thread GitBox 
mistercrunch commented on a change in pull request #4463: New Landing Page v1.0
URL: 
https://github.com/apache/incubator-superset/pull/4463#discussion_r170330854
 
 

 ##
 File path: superset/assets/javascripts/welcome/App.jsx
 ##
 @@ -17,24 +23,65 @@ export default class App extends React.PureComponent {
   render() {
 return (
   
-
-  
-Dashboards
-
-  
-
-  
-  
-  
-
+
+  
+
+  
+Recent Viewed
+  
+  
+  
+
+  
+  
+
+  
+Favorites
+  
+  
+  
+
+  
+  
+
+  
+Dashboards
+  
+  
+  
+
+  
+  
+
+  
+Datasources
+  
+  
+  {/*  */}
 
 Review comment:
   Why the empty placeholders? I'm thinking it's better to not have the empty 
tabs than having them.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mistercrunch commented on a change in pull request #4463: New Landing Page v1.0

2018-02-23 Thread GitBox 
mistercrunch commented on a change in pull request #4463: New Landing Page v1.0
URL: 
https://github.com/apache/incubator-superset/pull/4463#discussion_r170331102
 
 

 ##
 File path: superset/assets/javascripts/welcome/App.jsx
 ##
 @@ -17,24 +23,65 @@ export default class App extends React.PureComponent {
   render() {
 return (
   
-
-  
-Dashboards
-
-  
-
-  
-  
-  
-
+
+  
 
 Review comment:
   Lots of free text that should be wrap for i18n as in `t('Translate this')`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services