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.

Sign up for GitHub

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

Jump to bottom

Add home page #5494

Merged
merged 3 commits into from
Apr 21, 2020
Merged

Add home page #5494

merged 3 commits into from
Apr 21, 2020

Conversation

IKarbowiak
Copy link
Member

@IKarbowiak IKarbowiak commented Apr 17, 2020 •
edited
Loading

Add view for a splash page and create a template with links to the playground, dashboard, and storefront.

Impact

  • New migrations
  • New/Updated API fields or mutations
  • Deprecated API fields or mutations
  • Removed API types, fields, or mutations

Pull Request Checklist

  • Privileged queries and mutations are guarded by proper permission checks
  • Database queries are optimized and the number of queries is constant
  • Database migration files are up to date
  • The changes are tested
  • GraphQL schema and type definitions are up to date
  • Changes are mentioned in the changelog

@IKarbowiak IKarbowiak self-assigned this Apr 17, 2020
@lgtm-com
Copy link

lgtm-com bot commented Apr 17, 2020

This pull request introduces 2 alerts when merging 823df1d into 21af455 - view on LGTM.com

new alerts:

  • 2 for Potentially unsafe external link

@IKarbowiak IKarbowiak mentioned this pull request Apr 17, 2020
Merged
@fowczarek
Copy link
Member

Are "STOREFRONT_URL" and "DASHBOARD_URL" set in saleor-platform?

@IKarbowiak
Copy link
Member Author

@fowczarek, there is PR for it right now: saleor/saleor-platform#24

fowczarek
fowczarek reviewed Apr 17, 2020
View reviewed changes
templates/home.html Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 17, 2020

This pull request introduces 2 alerts when merging 78c00b0 into 21af455 - view on LGTM.com

new alerts:

  • 2 for Potentially unsafe external link

tomaszszymanski129
tomaszszymanski129 reviewed Apr 20, 2020
View reviewed changes
saleor/core/views.py


def home(request):
storefront = os.environ.get("STOREFRONT_URL", "")
Copy link
Member

Choose a reason for hiding this comment

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

If we don't have those set (for ex. someone cloned saleor and running it outside of docker) that will result in broken link in template. Maybe worth adding localhost:3000 as second arg?

Copy link
Member Author

@IKarbowiak IKarbowiak Apr 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The links will not be broken, links to the dashboard and storefront won't appear in this situation. If we put there default links and somebody doesn't have running instances of the dash and store it will cause broken links.

@maarcingebala maarcingebala merged commit de7daa0 into feature/splash-page Apr 21, 2020
@maarcingebala maarcingebala deleted the add-start-view branch April 21, 2020 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomaszszymanski129 tomaszszymanski129 tomaszszymanski129 left review comments

@fowczarek fowczarek fowczarek approved these changes

@maarcingebala maarcingebala Awaiting requested review from maarcingebala

Assignees

@IKarbowiak IKarbowiak

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IKarbowiak @fowczarek @maarcingebala @tomaszszymanski129