Univention Bugzilla – Bug 46462
(ucs-test) run checks after pre-hooks, allow to remove email address
Last modified: 2018-04-17 13:43:44 CEST
We should add a specific test, that checks most/all combinations: - w/ and w/o CSV column - w/ and w/o mapping - w/ and w/o value within the CSV column - check pre-hooks after pyhook changed some values - values are valid/invalid → check should report errors +++ This bug was initially created as a clone of Bug #46317 +++ The ucs-school-user-import runs its consistency/syntax checks before the pre_{create,modify,move} hooks. That creates the problem that data modified in the hooks is not checked. In a customers case the email uniqueness checks triggers because the email address is moved from one user to another in a pre_modify hook. As the email address was available during the check with both users before the pre_modify hook, it was counted twice. Move the checks code after the pre_{create,modify,move} hooks, but before the actual {create,modify,move}().
I added two tests: * 229_import-users_blank_email checks all possible combinations of the email column/mapping/schema * 230_import_user_check_after_pre-hook verifies post pre-hook checks [4.3] a1ca807e Bug #46462: add tests for CSV email field behavior and checks after pre-hooks [4.3] f31daea8 Bug #46462: cleanup [4.3] f10e22bf Bug #46462: changelog ucs-test-ucsschool (5.0.2-25) [4.2] 1b4a70be Bug #46462: add tests for CSV email field behavior and checks after pre-hooks [4.2] 969a7dbc Bug #46462: cleanup [4.2] a9b5ea42 Bug #46462: changelog ucs-test-ucsschool (4.0.4-72) -------- 229_import-users_blank_email checks: 1.1.1 scheme and mapping and column filled, NEW users -> filled email addresses from column 1.1.2 scheme and mapping and column filled, SAME users with new email addresses -> new email addresses from column 1.1.3 scheme and mapping and column empty, SAME users -> empty email addresses from column 1.2.1 scheme and mapping and column empty, NEW users -> empty email addresses from column 1.2.2 scheme and mapping and column empty, SAME users -> empty email addresses from column 1.3.1 scheme and mapping and no column, NEW users (with email addresses in CSV) -> filled email addresses from scheme 1.3.2 scheme and mapping and no column, SAME users (with email addresses in CSV) -> email addresses unchanged from 1.3.1 1.3.3 scheme and mapping and column empty, SAME users -> empty email addresses from column 2.1.1 scheme and no mapping and column filled, NEW users -> filled email addresses from scheme 2.1.2 scheme and no mapping and column filled, SAME users -> email addresses unchanged from 2.1.1 2.2 scheme and no mapping and column empty, NEW users (with email addresses in CSV) -> filled email addresses from scheme 2.3 scheme and no mapping and no column, NEW users -> filled email addresses from scheme 3.1 no scheme and mapping and column filled, NEW users -> new email addresses from column 3.2 no scheme and mapping and column empty, NEW users -> empty email addresses (from column) 3.3 no scheme and mapping and no column, NEW users -> empty email addresses 4.1 no scheme and no mapping and column filled, NEW users -> empty email addresses 4.2 no scheme and no mapping and column empty, NEW users -> empty email addresses 4.3 no scheme and no mapping and no column, NEW users -> empty email addresses ----- 230_import_user_check_after_pre-hook revealed a bug introduced by commit 314d3e6 (incorrectly labeled for Bug #45640). The checks are run _after_ the move()! The fix is simple, but before working on it, I want to discuss it tomorrow. The test succeeds with the following patch: --------------------------------------------------------------------- --- /usr/share/pyshared/ucsschool/importer/models/import_user.py.ori 2018-03-22 16:46:26.916443000 +0100 +++ /usr/share/pyshared/ucsschool/importer/models/import_user.py 2018-03-22 16:46:40.284443000 +0100 @@ -693,20 +693,20 @@ self._lo = lo self.check_schools(lo) return super(ImportUser, self).move(lo, udm_obj, force) def move_without_hooks(self, lo, udm_obj, force=False): + self.run_checks(check_username=False) old_dn = self.old_dn res = super(ImportUser, self).move_without_hooks(lo, udm_obj, force) if res: # rewrite self._unique_ids, replacing old DN with new DN for category, entries in self._unique_ids.items(): for value, dn in entries.items(): if dn == old_dn: self._unique_ids[category][value] = self.dn - self.run_checks(check_username=False) return res @classmethod def normalize(cls, s): """
229_import-users_blank_email is green in 4.2 and 4.3 -> OK 230_import_user_check_after_pre-hook is green in 4.2 and 4.3 -> OK -> Verified
no release required → CLOSED