Moved login/logout logic into the useUser composable and updated references #12915

iamshobhraj · 2024-12-06T12:49:50Z

Summary

Login/logout and session logic is moved into useUser composable from action.js. Refactored vuex state into module level state. References to CORE_SET_SESSION is updated to use the setSession method from useUser composable.

References

fixes #12204

Reviewer guidance

after updating references to CORE_SET_SESSION in heartbeat.spec.js file, some tests still failed. I am unable to figure out why these tests are failing. I updated the mock file and implementated it as written in documentation.

github-actions · 2024-12-06T13:20:56Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0a0.dev0_git.383.gefb4f32e.pex
Windows Installer (EXE)	kolibri-0.18.0a0.dev0+git.383.gefb4f32e-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0a0.dev0+git.383.gefb4f32e-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0a0.dev0+git.383.gefb4f32e.dmg
Android Package (APK)	kolibri-0.18.0a0.dev0+git.383.gefb4f32e-0.1.4-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.18.0a0.dev0+git.383.gefb4f32e.zip
TAR file	kolibri-0.18.0a0.dev0+git.383.gefb4f32e.tar.gz
WHL file	kolibri-0.18.0a0.dev0+git.383.gefb4f32e-py2.py3-none-any.whl

nucleogenesis · 2024-12-09T20:05:26Z

packages/kolibri/__tests__/heartbeat.spec.js


 describe('HeartBeat', function () {
  stubWindowLocation(beforeAll, afterAll);
  // replace the real XHR object with the mock XHR object before each test
-  beforeEach(() => mock.setup());
+  beforeEach(() => useUser.mockImplementation(() => useUserMock()));


Is there a particular reason for mock.setup() not to be called anymore? I'm wondering if this is related to the failing tests

Sorry for the oversight. I’ve fixed this now.

nucleogenesis

In the description you mentioned that the logic was moved from an actions.js file but there are no changes to any Vuex files included.

I also see that heartbeat.js may need to be updated to use the useUser module as it is still calling dispatch('setSession', ...) here.

I had a comment in the heartbeat.spec.js file re: the tests but I think that updating heartbeat.js might be the key

iamshobhraj · 2024-12-10T13:32:56Z

In the description you mentioned that the logic was moved from an actions.js file but there are no changes to any Vuex files included.

@nucleogenesis Thanks for pointing that out. I had initially planned on removing the Vuex logic after successfully migrating the login and session logic to the new Composition API. However, I got stuck with the heartbeat.spec.js tests. I’ve now removed the login/logout and session logic from the Vuex files.

I also see that heartbeat.js may need to be updated to use the useUser module as it is still calling dispatch('setSession', ...) here.

I had a comment in the heartbeat.spec.js file re: the tests but I think that updating heartbeat.js might be the key

Regarding the heartbeat.js, I refactored the store.dispatch to use the setSession method, but the failing tests persist. I suspect the issue might be related to xhr-mock, but I’m unable to pinpoint the exact cause.

Please let me know if you have any suggestions.

nucleogenesis

I added one comment to the mock function for setSession and after some thinking and looking at the code locally I'm not sure precisely what the fix is, but I'm feeling it is very likely to do with that function altogether and it's differences to the real setSession.

Another thing I saw was that the kolibriLogin action is being dispatched a few places - such as the store.spec.js file

Thank you for your effort on this. I hope anything I've said can be helpful. I'll keep thinking about this and come back to it again tomorrow if I can

nucleogenesis · 2024-12-13T00:27:23Z

packages/kolibri/composables/__mocks__/useUser.js

+    session, // Make session mutable for test scenarios
+
+    // Actions
+    setSession: jest.fn(({ session: newSession, clientNow }) => {


One of the failing tests re: not calling serverTime w/ clientNow may be fixed by calling it here.

rtibbles · 2025-01-09T03:01:15Z

When I did a rebase locally on develop here, it threw up some weird conflicts, but resolving them during the rebase did reduce it down to only about 10 commits, so I think this can be cleaned up! Let me know if it would be helpful for me to force push updates for this.

iamshobhraj · 2025-01-13T07:21:19Z

When I did a rebase locally on develop here, it threw up some weird conflicts, but resolving them during the rebase did reduce it down to only about 10 commits, so I think this can be cleaned up! Let me know if it would be helpful for me to force push updates for this.

Thank you for your help! If you think force pushing the updates would clean up the commit history and make it easier to manage, I would appreciate it. Please go ahead and do so if it's not too much trouble.

rtibbles · 2025-01-16T17:56:58Z

Sure thing, @iamshobhraj - I will make the update for you!

…osition API useUser

rtibbles

There are still a few failing tests after the rebase, but the commit history seems a bit more coherent now.

rtibbles · 2025-01-21T17:29:10Z

packages/kolibri/composables/useUser.js

@@ -52,15 +145,11 @@ export default function useUser() {
    getUserKind,
    userHasPermissions,
    session,
-    //state


Did you cross check that these are not used anywhere before deleting them?

rtibbles · 2025-01-21T17:31:03Z

packages/kolibri/composables/useUser.js

+    };
+  }
+
+  async function kolibrisetUnspecifiedPassword({ username, password, facility }) {


This doesn't need to be in here, we can leave it in the actions.js instead.

rtibbles · 2025-01-21T17:31:24Z

packages/kolibri/heartbeat.js

@@ -196,17 +196,8 @@ export class HeartBeat {
            redirectBrowser();
          }
        }
-        store.dispatch('setSession', {
-          session,
-          // Calculate an approximation of the client 'now' that was simultaneous to the server


This explanatory comment should not be deleted.

MisRob requested review from rtibbles and nucleogenesis December 9, 2024 08:57

MisRob added the TODO: needs review Waiting for review label Dec 9, 2024

nucleogenesis reviewed Dec 9, 2024

View reviewed changes

nucleogenesis requested changes Dec 9, 2024

View reviewed changes

github-actions bot added the DEV: frontend label Dec 10, 2024

nucleogenesis reviewed Dec 13, 2024

View reviewed changes

rtibbles self-assigned this Jan 16, 2025

iamshobhraj and others added 7 commits January 21, 2025 09:12

moved login/logout logic into useUser composable

bc596f5

updated references to CORE_SET_SESSION and changed it to the new comp…

d89cfdd

…osition API useUser

updated useUser mock file as per the new compostion api

2390b48

added 'mock.setup' in 'beforeEach'

6af778d

[pre-commit.ci lite] apply automatic fixes

942cbf5

removed existing vuex methods

74e76e5

refactored store.dispatch with setSession mutation

dcdd3f7

iamshobhraj and others added 9 commits January 21, 2025 09:12

removed existing vuex methods

ca263d7

resolved merge conflict

47ec259

fixed imports for ref and computed from vue

17e4abe

fix useUser mock to handle clientNow in setSession

361e0bb

updated references to CORE_SET_SESSION and changed it to the new comp…

1d47ceb

…osition API useUser

updated useUser mock file as per the new compostion api

9bb9d8d

[pre-commit.ci lite] apply automatic fixes

77f8db6

removed existing vuex methods

835fc65

fixed linting errors

f330911

rtibbles force-pushed the useUser-comp branch from 68f29f5 to f330911 Compare January 21, 2025 17:28

[pre-commit.ci lite] apply automatic fixes

ce857d9

rtibbles requested changes Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved login/logout logic into the useUser composable and updated references #12915

Moved login/logout logic into the useUser composable and updated references #12915

iamshobhraj commented Dec 6, 2024

github-actions bot commented Dec 6, 2024 •

edited

Loading

nucleogenesis Dec 9, 2024

iamshobhraj Dec 10, 2024

nucleogenesis left a comment

iamshobhraj commented Dec 10, 2024

nucleogenesis left a comment

nucleogenesis Dec 13, 2024

rtibbles commented Jan 9, 2025

iamshobhraj commented Jan 13, 2025

rtibbles commented Jan 16, 2025

rtibbles left a comment

rtibbles Jan 21, 2025

rtibbles Jan 21, 2025

rtibbles Jan 21, 2025

Moved login/logout logic into the useUser composable and updated references #12915

Are you sure you want to change the base?

Moved login/logout logic into the useUser composable and updated references #12915

Conversation

iamshobhraj commented Dec 6, 2024

Summary

References

Reviewer guidance

github-actions bot commented Dec 6, 2024 • edited Loading

Build Artifacts

nucleogenesis Dec 9, 2024

Choose a reason for hiding this comment

iamshobhraj Dec 10, 2024

Choose a reason for hiding this comment

nucleogenesis left a comment

Choose a reason for hiding this comment

iamshobhraj commented Dec 10, 2024

nucleogenesis left a comment

Choose a reason for hiding this comment

nucleogenesis Dec 13, 2024

Choose a reason for hiding this comment

rtibbles commented Jan 9, 2025

iamshobhraj commented Jan 13, 2025

rtibbles commented Jan 16, 2025

rtibbles left a comment

Choose a reason for hiding this comment

rtibbles Jan 21, 2025

Choose a reason for hiding this comment

rtibbles Jan 21, 2025

Choose a reason for hiding this comment

rtibbles Jan 21, 2025

Choose a reason for hiding this comment

github-actions bot commented Dec 6, 2024 •

edited

Loading