Skip to content

[Improvement] Redirect to login page if user doesn't login#3274

Merged
zhoujinsong merged 5 commits intoapache:masterfrom
chouchouji:fix-login
Dec 2, 2024
Merged

[Improvement] Redirect to login page if user doesn't login#3274
zhoujinsong merged 5 commits intoapache:masterfrom
chouchouji:fix-login

Conversation

@chouchouji
Copy link
Contributor

Why are the changes needed?

Close #xxx.

Brief change log

20241017-212425.mp4

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@github-actions github-actions bot added the module:ams-dashboard Ams dashboard module label Oct 17, 2024
@zhoujinsong
Copy link
Contributor

Thanks for the work!

When I tested it in my local environment, I got some unexpected errors:
When I try to open http://127.0.0.1:1630/overview page without logging, it can redirect to the login page, but after I fill in the username and password and click sign in, it cannot redirect to the overview page.

@chouchouji
Copy link
Contributor Author

logging

It may be a bug, I will fix it in recent days.

@majin1102
Copy link
Contributor

majin1102 commented Oct 21, 2024

Thanks for this contribution.

It really helps much.
I used to meet this issue
img_v3_02fj_47eaa5b9-fd87-4e32-a86e-e025eea48d6g

When backend restarted, refresh would encounter this plenty of errors. Will they disappear after this PR merged? @chouchouji

@czy006
Copy link
Contributor

czy006 commented Oct 30, 2024

+1

@czy006
Copy link
Contributor

czy006 commented Nov 26, 2024

Does this have anything to do with it #3340 cc @huyuanfeng2018

@huyuanfeng2018
Copy link
Contributor

Does this have anything to do with it #3340 cc @huyuanfeng2018

I'm not sure, I wasn't focused here

@baiyangtx
Copy link
Contributor

Does this have anything to do with it #3340 cc @huyuanfeng2018

This PR does not conflict with the current PR. The two PRs focus on different points respectively and solve different problems. @czy006

Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhoujinsong zhoujinsong merged commit b6b1e0f into apache:master Dec 2, 2024
@zhangmo8 zhangmo8 mentioned this pull request Dec 2, 2024
3 tasks
@chouchouji chouchouji deleted the fix-login branch December 12, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-dashboard Ams dashboard module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants