Implement all_objects manager class in FacilityUser #13433

ozer550 · 2025-05-29T11:07:38Z

Summary

Add the all_objects manager class for FacilityUsers.
Implement all_objects call in bulk import.
Unit tests.

References

closes #13376

Reviewer guidance

We would want to confirm that all the existing features for bulkImport work as expected.
Especially need to test that the soft deleted user is undeleted when we import it using the csv.
Steps to Reciprocate:
unzip the sqlite file in KOLIBRI_HOME folder. (This has soft deleted user a)
Restore the users using the linked csv.
check that user "a" is restored.
db_with_deleted_users.zip
ozer_2420_users.csv

github-actions · 2025-05-29T11:41:17Z

Build Artifacts

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

nucleogenesis

Looks good so far although it may require some coordination with Alex's .objects PR.

nucleogenesis · 2025-05-29T13:31:15Z

kolibri/core/auth/test/test_models.py

+            password=make_password("password"),
+            facility=self.facility,
+        )
+        deleted_user.delete()


.delete here will actually remove the user from the db rather than "soft-deleting" them. If you want to soft-delete the user you'll need to set its date_deleted field to a date.

nucleogenesis · 2025-05-29T13:33:58Z

kolibri/core/auth/test/test_models.py

+        )
+        deleted_user.delete()
+
+        self.assertEqual(FacilityUser.objects.count(), 1)


This line won't work until Alex's PR changing the .objects queryset is merged.

ozer550 · 2025-06-03T10:57:13Z

Would need QA on this @pcenov @radinamatic specifically on bulkUserImport features and the feature that import from csv undeletes soft deleted_user.

LianaHarris360

The changes in the models file look good, I just had a few thoughts on the updated tests.

LianaHarris360 · 2025-06-03T19:50:21Z

kolibri/core/auth/test/test_models.py

+        self.facility = Facility.objects.create(name="My Facility")
+
+    def test_all_objects_manager_returns_all_users(self):
+        deleted_user = FacilityUser.objects.create(


Just noting that you can set the date_deleted field when the user is initially created as well. Then you wouldn't have to set it below and save the change separately.

LianaHarris360 · 2025-06-03T19:54:42Z

kolibri/core/auth/test/test_models.py

+    def test_all_objects_manager_returns_all_users(self):
+        deleted_user = FacilityUser.objects.create(
+            username="deleted_user",
+            password=make_password("password"),


Is there a particular reason to use Django's make_password for password hashing here? Mostly wondering because the other passwords in this file are just set as "password".

ahh seems like this was a co-pilot tab complete that I overlooked.

LianaHarris360 · 2025-06-03T20:02:36Z

kolibri/core/auth/test/test_models.py

@@ -662,6 +663,28 @@ def test_default_facility_returns_none_when_no_settings(self):


 class FacilityUserTestCase(TestCase):
+    def setUp(self):
+        self.facility = Facility.objects.create(name="My Facility")


I think it makes sense to create the facility in a setup function when that facility is expected to be reused, but in this case, it should be fine to follow the pattern of the other functions below and use the line self.facility = Facility.objects.create() directly within test_all_objects_manager_returns_all_users()

pcenov · 2025-06-04T12:36:16Z

Hi @ozer550 I am getting the following error when I try to start Kolibri after having replaced the db.sqlite3 file with the one provided in the PR:

osboxes@osboxes:~$ kolibri start
INFO     2025-06-04 15:33:38,064 Option DEBUG in section [Server] being overridden by environment variable KOLIBRI_DEBUG
INFO     2025-06-04 15:33:38,065 Option DEBUG_LOG_DATABASE in section [Server] being overridden by environment variable KOLIBRI_DEBUG_LOG_DATABASE
INFO     2025-06-04 15:33:38,065 Option RUN_MODE in section [Deployment] being overridden by environment variable KOLIBRI_RUN_MODE
Error: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/kolibri/utils/cli.py", line 199, in invoke
    initialize(**get_initialize_params())
  File "/usr/lib/python3/dist-packages/kolibri/utils/main.py", line 317, in initialize
    _upgrades_before_django_setup(updated, version)
  File "/usr/lib/python3/dist-packages/kolibri/utils/main.py", line 237, in _upgrades_before_django_setup
    sqlite_check_foreign_keys()
  File "/usr/lib/python3/dist-packages/kolibri/utils/main.py", line 196, in sqlite_check_foreign_keys
    cursor.execute("PRAGMA foreign_key_check;")
sqlite3.DatabaseError: database disk image is malformed

Could you have a look and let me know how we can proceed? Thanks!

radinamatic · 2025-06-04T12:43:35Z

@pcenov Could you try following these steps from our user docs, and see if with them you manage to fix the malformed DB?

https://kolibri.readthedocs.io/en/latest/manage/get_support.html#malformed-database

pcenov · 2025-06-04T13:47:00Z

The steps in https://kolibri.readthedocs.io/en/latest/manage/get_support.html#malformed-database are not working for this case where we are intentionally replacing the DB with the one provided in the PR.
Hopefully @ozer550 will provide more clarity on the matter, any additional points for the manual QA required here will be greatly appreciated too!

LianaHarris360 · 2025-06-04T14:48:43Z

~~I had the same problem as Peter. Do we maybe need the entire KOLIBRI_HOME folder instead of only the sqlite file?~~
I was able to restore a "soft" deleted user using a CSV import, it required writing some python code within the terminal. I can confirm that the all_objects call in bulk import works as expected.

ozer550 · 2025-06-04T15:46:56Z

Hi @pcenov and @radinamatic it seems like its a little more complicated then just sharing the database file to reciprocate the state of my kolibri. I synced up with @LianaHarris360 and specifically walked her thorough regression testing the restoration of soft deleted user. So I think if @pcenov can now regression test on other features related with bulk import, we would be quite sure that we have whats expected.

ozer550 · 2025-06-04T15:52:07Z

To clarify more, the only way to currently soft delete the user is to log in thru shell and then run a query to soft delete the user. I thought that sharing the database file would actually replicate the soft deleted user for you and you would not need to run those queries, but seems like it doesn't work that way. If we would want to document exact steps on how I regression tested this, I could write some docs if that would be helpful.

pcenov · 2025-06-09T13:36:03Z

Thanks @ozer550 - no regressions observed while manually testing the bulk import.

LianaHarris360

This should be good to merge in now!

github-actions bot added SIZE: small DEV: backend Python, databases, networking, filesystem... labels May 29, 2025

nucleogenesis reviewed May 29, 2025

View reviewed changes

ozer550 force-pushed the implement-all-objects-manager-class branch from e9d5f4f to b553c3b Compare June 2, 2025 17:27

github-actions bot added the SIZE: medium label Jun 2, 2025

ozer550 added 3 commits June 3, 2025 11:26

implemented all_objects manager class

a02ccb4

change all_objects in bulkImport

f263aa6

fix failing tests

4c08b3e

ozer550 force-pushed the implement-all-objects-manager-class branch from b553c3b to 4c08b3e Compare June 3, 2025 05:57

ozer550 marked this pull request as ready for review June 3, 2025 10:46

fix failing tests

bdb9608

ozer550 changed the title ~~implemented all_objects manager class~~ Implement all_objects manager class in FacilityUser Jun 3, 2025

migrate database

c2fa937

ozer550 requested a review from nucleogenesis June 3, 2025 11:14

LianaHarris360 self-assigned this Jun 3, 2025

LianaHarris360 reviewed Jun 3, 2025

View reviewed changes

make slight changes to tests

1106c0c

ozer550 requested a review from LianaHarris360 June 4, 2025 06:32

nucleogenesis added this to the Kolibri 0.19: Bulk User Management milestone Jun 6, 2025

LianaHarris360 approved these changes Jun 9, 2025

View reviewed changes

LianaHarris360 merged commit c4ca571 into learningequality:develop Jun 9, 2025
51 checks passed

Implement all_objects manager class in FacilityUser #13433

Implement all_objects manager class in FacilityUser #13433

Uh oh!

Conversation

ozer550 commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

Uh oh!

github-actions bot commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

nucleogenesis left a comment

Choose a reason for hiding this comment

Uh oh!

nucleogenesis May 29, 2025

Choose a reason for hiding this comment

Uh oh!

nucleogenesis May 29, 2025

Choose a reason for hiding this comment

Uh oh!

ozer550 commented Jun 3, 2025

Uh oh!

LianaHarris360 left a comment

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

ozer550 Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

LianaHarris360 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

pcenov commented Jun 4, 2025

Uh oh!

radinamatic commented Jun 4, 2025

Uh oh!

pcenov commented Jun 4, 2025

Uh oh!

LianaHarris360 commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ozer550 commented Jun 4, 2025

Uh oh!

ozer550 commented Jun 4, 2025

Uh oh!

pcenov commented Jun 9, 2025

Uh oh!

LianaHarris360 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ozer550 commented May 29, 2025 •

edited

Loading

github-actions bot commented May 29, 2025 •

edited

Loading

LianaHarris360 commented Jun 4, 2025 •

edited

Loading