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

Feature/login #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Feature/login #2

wants to merge 3 commits into from

Conversation

hayashi1917
Copy link
Collaborator

受講生の確認事項

  • [x ] 画面をブラウザで実際に開いてテスト要件の画面と機能の動作確認をした(動作が分からない場合講師からスクリーンショットの提出を求めることがあります)
  • [x ] 作成したモデルを全てDjango管理画面に登録した
  • [x ] テスト要件のテストを全て実装した
  • [x ] CI が全て通った

1次レビュアーの確認事項

@Yoshida-koshi
Copy link

success_url = reverse_lazy("tweets:home")

ここのtweets:homeをsettings.pyでLOGIN_REDIRECT_URLに設定して、このLOGIN_REDIRECT_URLを呼び出すように修正してみてください!

@Yoshida-koshi
Copy link

上のリンクでページに飛べるところにLoginのリンクを追加して、ログインページに簡単に飛べるようにしてみると、ユーザーがもっと使いやすくなると思います!

スクリーンショット 2024-06-29 22 49 10

@Yoshida-koshi
Copy link

login.htmlはaccountsフォルダに入れて欲しいです!

Copy link

@Yoshida-koshi Yoshida-koshi left a comment

Choose a reason for hiding this comment

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

コメントしました〜

# path('<str:username>/', views.UserProfileView.as_view(), name='user_profile'),
path("login/", auth_views.LoginView.as_view(), name="login"),
path("logout/", auth_views.LogoutView.as_view(), name="logout"),
path("profile/", views.UserProfileView.as_view(), name="user_profile"),

Choose a reason for hiding this comment

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

ここのエンドポイントは最終課題テスト項目要件整理に沿って設定してほしいです!


LOGIN_REDIRECT_URL = '/tweets/home'
LOGIN_REDIRECT_URL = "/accounts/profile/"

Choose a reason for hiding this comment

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

ここはtweets:homeにした方がいいと思います!

@@ -127,8 +127,8 @@

DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField"

LOGIN_URL = '/login'
LOGIN_URL = "/accounts/login/"

Choose a reason for hiding this comment

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

LOGIN_URL, LOGIN_REDIRECT_URL, LOGOUT_REDIRECT_URLは/〇〇/〇〇の形ではなく、〇〇:〇〇にして欲しいです!

@@ -7,7 +7,7 @@
{{ form.as_p }} <!--← formにはSignupFormのインスタンスが入っている。as_pとすることで、各input要素がpタグで囲まれた状態で表示される。-->
{% csrf_token %} <!--← csrf_token必須。formタグ内であればどこに書いてもOK -->
<button type="submit">ユーザー登録</button>
</form>
</form>

Choose a reason for hiding this comment

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

ここのインデントはいらないと思います!

self.assertEqual(response.status_code, 200)
self.assertFalse(form.is_valid())
self.assertIn("このフィールドは必須です。", form.errors["username"])
self.assertIn("このフィールドは必須です。", form.errors["password"])

Choose a reason for hiding this comment

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

ここの下にSESSION_KEYがclient.sessionに含まれているかのチェックを入れて欲しいです!


# def test_failure_post_with_not_exists_user(self):
def test_success_post(self):
User.objects.create_user(username="testuser", password="testpassword")

Choose a reason for hiding this comment

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

Userモデルの作成はsetUp関数で行って欲しいです!


class TestLogoutView(TestCase):
def test_success_post(self):
self.client.login(username="testuser", password="testpassword")

Choose a reason for hiding this comment

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

TestLogoutViewにsetUp関数を作成し、Userモデルの作成とログイン処理をそこに移動して欲しいです!

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