Fixes #5185 - Access settings page directly from url #5207
Conversation
punamdahiya
force-pushed
the
5185-settings-signin
branch
from
c9a70a0
to
90cef3b
Compare
punamdahiya
force-pushed
the
5185-settings-signin
branch
from
90cef3b
to
55a4a35
Compare
|
As discussed #5185 (comment) , for now if a user access /settings from url without signin, we are redirecting user to homepage. |
test/server/test_responses.py
Outdated
| @@ -45,7 +45,7 @@ def test_settings_page_requires_auth(): | |||
| user = ScreenshotsClient() | |||
|
|
|||
| resp = requests.get(urljoin(user.backend, "/settings")) | |||
| assert resp.status_code == 403 | |||
| assert resp.status_code == 200 | |||
There was a problem hiding this comment.
We should change this test to confirm that the response was redirected (otherwise the test no longer really tests anything, i.e., doesn't test authentication logic). This is a response object and an be checked for resp.is_redirect or probably better would be to check resp.url == user.backend + "/"
| @@ -75,20 +75,12 @@ class Body extends React.Component { | |||
| <Localized id="settingsGuestAccountMessage"> | |||
| <p className="title">Guest Account</p> | |||
| </Localized> | |||
| <Localized id="settingsSignInInvite"> | |||
| <p className="info">Sign in to sync across devices</p> | |||
| </Localized> | |||
There was a problem hiding this comment.
It doesn't seem right to me that we are removing these...? Don't we still want to display these messages to people who aren't logged in via FxA?
There was a problem hiding this comment.
We are redirecting user to homepage when a user doesn't have accountId cookie set.
This block of code shows only if accountInfo is null that is querying db for the accountId doesn't return any record.
If needed for this edge case, we can keep the strings with word sync removed from messaging.
There was a problem hiding this comment.
Updated to bring string back after removing sync mesaging
Settings page with no account Info
Settings page with account Info
punamdahiya
force-pushed
the
5185-settings-signin
branch
from
55a4a35
to
635ef90
Compare
|
Including @flodolo to review string changes. Thanks! |
No description provided.