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

Develop #792

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Develop #792

wants to merge 7 commits into from

Conversation

ihorutkin
Copy link

No description provided.

@ihorutkin
Copy link
Author

image
image
image
image
image
image

@@ -29,7 +29,7 @@ class Migration(migrations.Migration):
('last_name', models.CharField(blank=True, max_length=150, verbose_name='last name')),
('email', models.EmailField(blank=True, max_length=254, verbose_name='email address')),
('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')),
('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')),
Copy link

Choose a reason for hiding this comment

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

do you need this change?
you should use auto replace carefully and check what it changes before applying changes because migrations may fail ;)

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it was casually

@ihorutkin ihorutkin requested a review from MNDonut September 13, 2024 10:57
Copy link

@MNDonut MNDonut left a comment

Choose a reason for hiding this comment

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

I see you pushed db.sqlite3 and migration was not reverted

@ihorutkin ihorutkin requested a review from MNDonut September 16, 2024 08:58
Copy link

@MNDonut MNDonut left a comment

Choose a reason for hiding this comment

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

You didn't push any new commits. Db is still present and migration is not reverted back
image

@ihorutkin ihorutkin requested a review from MNDonut September 16, 2024 13:27
@@ -22,4 +22,8 @@
urlpatterns = [
path("admin/", admin.site.urls),
path("", include("taxi.urls", namespace="taxi")),
] + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)
path("registration/", include("django.contrib.auth.urls")),

Choose a reason for hiding this comment

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

I wonder how this path works for you without setting LOGIN_URL in settings 🤔
Default path for auth urls in django is accounts/ and it will not resolve default login/logout with a different name unless you specify LOGIN_URL

Copy link

@Oleksl888 Oleksl888 left a comment

Choose a reason for hiding this comment

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

Approved, but please revert changes to migration file

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.

3 participants