fix: add token expired message and fix navigation and title text #119
Conversation
techno-disaster
added
Category: User Interface
techno-disaster
requested review from
bartekpacia and
isabelcosta
as code owners
techno-disaster
changed the title
|
@anitab-org/qa-team can someone give this a quick test? |
There was a problem hiding this comment.
I'm basing my review on the GIF, but I would like @bartekpacia 's input on this.
isabelcosta
added
Status: Needs Testing
There was a problem hiding this comment.
I have tested this pr , it requires some changes as can be seen below :
The changes made in this PR were tested locally. Following are the results:
1. Code Review : Done
2. All possible responses were tested as below :
Screenshot/gif :
Expected Result : When I change a screen through clicking the bottom navigation menu the screen titles should change accordingly.
Actual Result : Not working in the expected manner. Title text remains the same even on navigating to different tabs at bottom
Expected Result : On clicking Find Members, it should lead you to the Members Page
Actual Result : When I first clicked it after I was just logged in then it was working fine and leading me to the Members Page but after again clicking it, it's not working and not leading anywhere.
3. Remaining Test Case : For testing the token I need to wait for 7 days till it expires. I Will update on it after Week
4. Additional test cases covered : n/a
5. Device : Emulator Pixel 2 XL API 27
|
@techno-disaster @isabelcosta didn't tested the first test case of token expire message as for this i have to wait for 7 days till it expires 😅 will update after a week . |
you can host the backup on your laptop and change that time. |
robotjellyzone
added
the
Status: Changes Requested
rpattath
removed
the
Status: Needs Testing
@robotjellyzone are you sure its not working? can you try pulling the latest changes from my branch just to be sure? I just tested this on my device again and it works just fine. |
|
@robotjellyzone any updates? |
|
@anitab-org/qa-team @anitab-org/mentorship-flutter-maintainers any updates? |
techno-disaster
added
Status: Needs Review
|
@anitab-org/mentorship-flutter-maintainers @anitab-org/coding-team can someone review this? |
hi, @techno-disaster can you please provide an account to check for this members button as it's not there due to the already present relation on the relation page ? |
Hey, I don't have one right now, but could you create one? If not I can get back to you by tommorow. Thanks |
ok i will create new as i have already created many so, sometimes creates confusion but ok I will create one now :) |
same here 🤦 . Thanks for the cooperation tho :) |
it will be great if you can test the token expiry feature by yourself :) as it requires a time span to be given .!! |
There was a problem hiding this comment.
I have tested this pr , it is working fine on my side :
The changes made in this PR were tested locally. Following are the results:
1. Code Review : Done
2. All possible responses were tested as below :
Screenshots gif :
Expected Result : When I change a screen through clicking the bottom navigation menu the screen titles should change accordingly.
Actual Result : Working as expected!
Expected Result : On clicking Find Members, it should lead you to the Members Page
Actual Result : Working as expected!
3. Remaining Test Case : For testing the token I need to wait for 7 days till it expires. It will be great if you @techno-disaster can test this feature will be provided by you
4. Additional test cases covered : n/a
5. Device : Emulator Pixel 2 XL API 27
|
@robotjellyzone thank you so so much for testing this 🙌 And about the token expiry test, I can always see how that goes in the future. I'll test that eventually. |
There was a problem hiding this comment.
@techno-disaster I noticed 2 things in the code, but I am only requesting one change, which is in the message displayed the user. I should have noticed this before 🙈 The change then won't need to have additional tests :)
| @@ -94,7 +95,12 @@ class _StatsPageState extends State<StatsPage> { | |||
| ); | |||
| } | |||
| if (state is StatsPageFailure) { | |||
| return Text(state.message); | |||
| if (state.message == "The token has expired! Please, login again or refresh it.") { | |||
There was a problem hiding this comment.
We have to think of a better way to do this, comparing the message will cause this feature to break if we decide later to change the message returned by the API. Perhaps now makes sense to return something else added to the message. Can you create an issue on Flutter repo to address this change, please? If not changed now perhaps later, but with an issue, we won't forget about this problem. @techno-disaster
There was a problem hiding this comment.
@anitab-org/mentorship-android-maintainers any idea how this is done on the android side? Not sure if there is any other alternative right now
There was a problem hiding this comment.
Not sure, but you can ask on #mentorship-system stream on Zulip. I don't remember now 🤔
isabelcosta
added
Status: Changes Requested
Co-authored-by: Isabel Costa <11148726+isabelcosta@users.noreply.github.com>
techno-disaster
added
Status: Needs Review
Description
This PR fixes the issue where the jwt token is expired and the user has to relogin to fix this. This PR also fixes the issue where the user could not navigate properly after clicking on the "Find Members" button, this also fixes the issue where text in app titile wont change
Fixes #118
Fixes #113
Fixes #124
Flutter Channel:
Type of Change:
Delete irrelevant options.
How Has This Been Tested?
Physical Device
Checklist:
s