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

Unnecessary kolibri store in tests #11852

Merged

Conversation

nick2432
Copy link
Contributor

@nick2432 nick2432 commented Feb 9, 2024

Summary

This PR addresses issue #11725 by removing unnecessary uses of the core Kolibri store in JavaScript test files. With the introduction of custom theming, the theming state was initially added to the core Vuex state. However, the reactive theming state has been extracted from Vuex and now lives independently. Consequently, injecting the store into component tests is no longer needed.

References

fixes #11725

@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend SIZE: small labels Feb 9, 2024
@MisRob MisRob added the TODO: needs review Waiting for review label Feb 12, 2024
@rtibbles rtibbles self-assigned this Feb 12, 2024
@rtibbles
Copy link
Member

Excellent - this definitely removes some unnecessary references!

The main question now is just confirming there are no other places that we could do this cleanup. I will take a quick glance through your PR code checked out locally to double check!

@rtibbles
Copy link
Member

From a quick grep, I suspect there may be more instances we could tidy up here.

I searched for kolibri.coreVue.vuex.store in files named *.spec.js and looked at the Git blame for the import, and more than one tracked back to #4587 - could you confirm that you have checked all instances here?

@github-actions github-actions bot added DEV: renderers HTML5 apps, videos, exercises, etc. APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) labels Feb 14, 2024
@nick2432
Copy link
Contributor Author

@rtibbles i have made some changes

@rtibbles
Copy link
Member

Excellent, this seems to have much wider coverage!

@rtibbles rtibbles merged commit f66f87c into learningequality:develop Feb 21, 2024
31 checks passed
poju3185 pushed a commit to poju3185/kolibri that referenced this pull request Mar 15, 2024
* Use responsive composable instead of mixin to fix errors from the mixin.

* fix:unnecessary-Kolibri-store-in-tests

* Update CardGrid.vue

* Update CardGrid.vue

* fix:unnecessary-Kolibri-store-in-tests

* Update utils.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend DEV: renderers HTML5 apps, videos, exercises, etc. SIZE: small TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove unnecessary use of the core Kolibri store in tests
3 participants