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

Add out of time redirect #5751

Merged
merged 4 commits into from
Feb 9, 2024
Merged

Add out of time redirect #5751

merged 4 commits into from
Feb 9, 2024

Conversation

raksooo
Copy link
Member

@raksooo raksooo commented Feb 1, 2024

From what I could find the app doesn't redirect to the out of time view when the account expires. It
redirected first when the app received focus or navigating to the account view or main view. I've
also added a test that verifies this.

I've also improved the logging for debugging account expiry issues in the test framework.


This change is Reviewable

@raksooo raksooo force-pushed the add-out-of-time-state-update branch 2 times, most recently from fe34023 to b06c534 Compare February 2, 2024 16:03
@raksooo raksooo marked this pull request as draft February 5, 2024 08:12
@raksooo raksooo force-pushed the add-out-of-time-state-update branch from b06c534 to 0c3b11a Compare February 5, 2024 11:17
@raksooo raksooo marked this pull request as ready for review February 5, 2024 11:17
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r2, all commit messages.
Reviewable status: 2 of 3 files reviewed, all discussions resolved

@raksooo raksooo force-pushed the add-out-of-time-state-update branch from 0c3b11a to 21e7064 Compare February 6, 2024 08:04
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @raksooo)


gui/test/e2e/mocked/expired-account-error-view.spec.ts line 7 at r3 (raw file):

import { getBackgroundColor } from '../utils';
import { colors } from '../../../src/config.json';
// import { RoutePath } from '../../../src/renderer/lib/routes';

This seems like it can be removed 😊

Code quote:

// import { RoutePath } from '../../../src/renderer/lib/routes';

gui/test/e2e/mocked/expired-account-error-view.spec.ts line 46 at r3 (raw file):

//     });
//   })).toEqual(RoutePath.expired);
// });

This seems like it can be removed 😊

Code quote:

// test('App should show out of time view after running out of time', async () => {
//   const expiryDate = new Date();
//   expiryDate.setSeconds(expiryDate.getSeconds() + 4);

//   expect(await util.waitForNavigation(async () => {
//     await util.sendMockIpcResponse<IAccountData>({
//       channel: 'account-',
//       response: { expiry: expiryDate.toISOString() },
//     });
//   })).toEqual(RoutePath.expired);
// });

@raksooo raksooo force-pushed the add-out-of-time-state-update branch from 21e7064 to a92caa2 Compare February 6, 2024 15:58
Copy link
Member Author

@raksooo raksooo left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98)


gui/test/e2e/mocked/expired-account-error-view.spec.ts line 46 at r3 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

This seems like it can be removed 😊

I though I had made this non-commented :o Now it's fixed at least and I've also changed the test so that each test in this file gets it's own window to not be affected by each other.

@raksooo raksooo force-pushed the add-out-of-time-state-update branch 2 times, most recently from 90edaed to 39a75e9 Compare February 6, 2024 16:14
Copy link
Member Author

@raksooo raksooo left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98)


gui/test/e2e/mocked/expired-account-error-view.spec.ts line 7 at r3 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

This seems like it can be removed 😊

Uncommented, done!

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @raksooo)


gui/test/e2e/mocked/expired-account-error-view.spec.ts line 46 at r3 (raw file):

Previously, raksooo (Oskar Nyberg) wrote…

I though I had made this non-commented :o Now it's fixed at least and I've also changed the test so that each test in this file gets it's own window to not be affected by each other.

Nice!

@raksooo raksooo force-pushed the add-out-of-time-state-update branch 12 times, most recently from 0edec56 to 8870453 Compare February 7, 2024 21:28
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@raksooo raksooo force-pushed the add-out-of-time-state-update branch from 8870453 to 683fbbc Compare February 9, 2024 16:45
@raksooo raksooo merged commit 3bea8ae into main Feb 9, 2024
5 checks passed
@raksooo raksooo deleted the add-out-of-time-state-update branch February 9, 2024 16:50
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.

2 participants