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

Drop storefront 1.0 #5043

Merged
merged 53 commits into from Dec 17, 2019
Merged

Drop storefront 1.0 #5043

merged 53 commits into from Dec 17, 2019

Conversation

IKarbowiak
Copy link
Member

@IKarbowiak IKarbowiak commented Dec 6, 2019

Drop storefront 1.0 -> views, forms, templates, statics
Closes #5017
Closes #5046

Pull Request Checklist

  1. Privileged views and APIs are guarded by proper permission checks.
  2. All visible strings are translated with proper context.
  3. All data-formatting is locale-aware (dates, numbers, and so on).
  4. Database queries are optimized and the number of queries is constant.
  5. Database migration files are up to date.
  6. The changes are tested.
  7. GraphQL schema and type definitions are up to date.
  8. Changes are mentioned in the changelog.

@IKarbowiak IKarbowiak self-assigned this Dec 6, 2019
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@krzysztofwolski krzysztofwolski left a comment

Dropping packages, dropping code. I love it.

Copy link
Member

@fowczarek fowczarek left a comment

I think we can remove:

  • LOGIN_URL = "/account/login/"
  • PAGINATE_BY = 16
  • DASHBOARD_PAGINATE_BY = 30
  • DASHBOARD_SEARCH_LIMIT = 5
  • LOGOUT_ON_PASSWORD_CHANGE = False
  • SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL = True
  • SOCIAL_AUTH_USER_MODEL = AUTH_USER_MODEL
  • SOCIAL_AUTH_FACEBOOK_SCOPE = ["email"]
  • SOCIAL_AUTH_FACEBOOK_PROFILE_EXTRA_PARAMS = {"fields": "id, email"}
  • SOCIAL_AUTH_REDIRECT_IS_HTTPS = True
    form settings.py, and:
  • "saleor.account.backends.google.CustomGoogleOAuth2",
  • "saleor.account.backends.facebook.CustomFacebookOAuth2",
    from AUTHENTICATION_BACKENDS in settings.py.
    Can someone check settings too?
    @NyanKiyoshi @maarcingebala @krzysztofwolski @korycins

saleor/account/events.py Outdated Show resolved Hide resolved
@IKarbowiak IKarbowiak closed this Dec 11, 2019
@IKarbowiak IKarbowiak reopened this Dec 11, 2019
Copy link
Contributor

@NyanKiyoshi NyanKiyoshi left a comment

@IKarbowiak I pushed some changes to drop useless middleware, apps, context processors. They are not that heavy on the processing time but save between 15-25% of processing in my tests.

I also removed django-redis as it is no longer needed (no more session caching), django-social, and more–including npm dependencies that are no longer used.

@NyanKiyoshi NyanKiyoshi requested a review from fowczarek Dec 12, 2019
@@ -33,7 +33,6 @@ dist/
/node_modules/
Copy link
Member

@korycins korycins Dec 16, 2019

Choose a reason for hiding this comment

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

Do we still need this? and docs?

Copy link
Member Author

@IKarbowiak IKarbowiak Dec 17, 2019

Choose a reason for hiding this comment

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

We still use npm for build-emails, so probably node_modules must stay. But in case of docs for me, it looks like we can remove it. @maarcingebala, what do you think?

Copy link
Member

@maarcingebala maarcingebala Dec 17, 2019

Choose a reason for hiding this comment

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

Yes, this probably needs to stay for now since we still have some packages in package.json.



@app.task
def update_all_products_minimal_variant_prices_task():
Copy link
Member

@korycins korycins Dec 16, 2019

Choose a reason for hiding this comment

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

AFAIK this task was created to future usage from celery beat. But I am not sure. You can ask @derenio

Copy link
Contributor

@derenio derenio Dec 17, 2019

Choose a reason for hiding this comment

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

This is true, although I'm not sure what's the status of shipping saleor with something like celery beat for which this task was designed. I think we can remove it. When we implement the default periodic tasks schema we will have to think about the minimal variant price recalculations.

@maarcingebala maarcingebala merged commit d3ec3cb into master Dec 17, 2019
2 of 4 checks passed
@maarcingebala maarcingebala deleted the drop-storefront-1.0 branch Dec 17, 2019
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.

9 participants