Bug 48138 - Automatically accept birthday in dd.mm.yy (28.09.18) format
Automatically accept birthday in dd.mm.yy (28.09.18) format
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.3
Other Mac OS X 10.1
: P5 normal (vote)
: UCS@school 4.4 v5-errata
Assigned To: Toni Röhmeyer
Daniel Tröder
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-12 21:26 CET by Michel Smidt
Modified: 2020-07-30 15:12 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.114
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Usability
Max CVSS v3 score:


Attachments
birth format (16.59 KB, image/png)
2018-11-12 21:26 CET, Michel Smidt
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michel Smidt 2018-11-12 21:26:02 CET
Created attachment 9740 [details]
birth format

By default micorsoft excel 2013 & 2016 format birthdays in dd.mm.yy (28.09.18). Even if the opened csv-file contains a birthday in the format JJJJ-MM-TT (2018-09-28). This is a problem for users. It is not that easy to change. One workaround is to change the cell format but I suggest to automatically accept the birthday format dd.mm.yy (28.09.18).
Comment 1 Daniel Tröder univentionstaff 2018-11-13 08:07:42 CET
Also: If you have a US version of MS Office it'll be mm/dd/yy or mm/dd/yyyy.

We have format-hooks that can reformat the input data accordingly.

It would be easy to detect those formats. Additional formats can be configured by using/modifying a format-hook.

The question here is, if the detection code should go into the import code or a format hook.
Comment 2 Toni Röhmeyer univentionstaff 2020-03-06 12:56:17 CET
On user import, the given birthday now gets checked against the syntax.py class "date2".

The behavior was tested by importing test users for the birthday formats
ISO (YYYY-MM-DD) and German (DD.MM.YY)


The solution was pushed on branch troehmey/bug48138 with commit

commit a841af65247b121999358f93dea43ab8d090b915
Bug #48138: birthday now accepted in german format
Comment 3 Daniel Tröder univentionstaff 2020-03-12 19:52:21 CET
Please also parse:

* mm.dd.yyyy
* mm/dd/yy
* mm/dd/yyyy

I wrote a unit test that verifies the behavior of ImportUser.make_birthday():

[troehmey/bug48138 f4237e79b] Bug #48138: add unittest for birthday format


# ./249_import_user_birthday_format.py

249_import_user_birthday_format.py::test_birthday_formats[2082-09-22] PASSED
249_import_user_birthday_format.py::test_birthday_formats[10.05.1981] FAILED
249_import_user_birthday_format.py::test_birthday_formats[10/02/1971] FAILED
249_import_user_birthday_format.py::test_birthday_formats[12.07.40] PASSED
249_import_user_birthday_format.py::test_birthday_formats[12/08/42] FAILED
Comment 4 Toni Röhmeyer univentionstaff 2020-03-20 21:40:27 CET
Added accepted birthday formats to import_user.py
Date parser now implemented in new function "parse_birthday()" in import_user.py instead of
Comment 5 Toni Röhmeyer univentionstaff 2020-03-20 21:43:06 CET
Added accepted birthday formats to import_user.py
Date parser now implemented in new function "parse_birthday()" in import_user.py instead of using the "date2" class from syntax.py.

Solution pushed on branch troehmey/bug48138 with commit

commit eb52c92aeb596e9811c53f59a387ee50e679444b
Bug #48138: added accepted date formats


waiting for QA
Comment 6 Tobias Wenzel univentionstaff 2020-04-17 16:47:55 CEST
QA

[troehmey/bug48138] eb52c92ae Bug #48138: added accepted date formats
[troehmey/bug48138] f4237e79b Bug #48138: add unittest for birthday format
[troehmey/bug48138] a841af652 Bug #48138: birthday now accepted in german format


Unittest & manual testing -> Ok

249_import_user_birthday_format.py::test_birthday_formats[1919-07-03] PASSED
249_import_user_birthday_format.py::test_birthday_formats[21.05.2060] PASSED
249_import_user_birthday_format.py::test_birthday_formats[09/05/1940] PASSED
249_import_user_birthday_format.py::test_birthday_formats[09.08.31] PASSED
249_import_user_birthday_format.py::test_birthday_formats[10/21/73] PASSED

Code

- date_syntax import -> not used 
- Why do your if statements (if year <= 30) in parse_date differ from date2.parse (if year >= 70)?
- This code seems redundant, because of the condition if self.birthday in make_birthday:

if text is None:
    return ''

- Add  type-hint  type: (str) -> str in parse_date
- You can simplefy the regexes and use r'', e.g. 

re_4 = re.compile('^[0-9]{2}/[0-9]{2}/[0-9]{2}$')
re_5 = re.compile('^[0-9]{2}/[0-9]{2}/[0-9]{4}$')
-> 	re_4 = re.compile(r'^[0-9]{2}/[0-9]{2}/[0-9]{2, 4}$')

- Use two spaces before comment 
- Whitespaces around 0,0,0
Comment 7 Toni Röhmeyer univentionstaff 2020-04-17 17:50:17 CEST
Thank you for the annotations!

I fixed the mentioned issues with commits

commit b4c346d8faeeb151a708673eb03df3a1f7db52c5
Bug #48138: fixed small code issues

commit b72a06df81bb4267f0a413aac85db94a676873fc
Bug #48138: fixed date issue

commit cfa090b97c1f5dce7c5297dfc930d8ff6246532b
Bug #48138: undo if condition change

on branch troehmey/bug48138.


The if conditions differ because this method is meant to parse birth dates.
E.g:
A person born in the 60s would otherwise get the birthdate 2060.

I changed the condition to "if year <= 21" so people born in 2020 (and next) year will still be accepted.
Comment 8 Tobias Wenzel univentionstaff 2020-04-20 10:27:34 CEST
Thanks for the quick response!

As discussed, you can use datetime.date.today().year. 
Before committing, run test 249 and make sure everything is green.

^[0-9]{2}\.[0-9]{2}\.[0-9]{2, 4}$ -> remove the whitespace {2,4}, otherwise it is interpreted as 2 or whitespace and 4.
Comment 9 Toni Röhmeyer univentionstaff 2020-04-20 13:45:04 CEST
I applied the suggested fixes in commit

commit 73300b974302d857d036031f694b2805fd78d4c0
Bug #48138: use current year in if condition

on branch troehmey/bug48138.

Test 249 passed successfully
Comment 10 Tobias Wenzel univentionstaff 2020-04-21 09:58:44 CEST
Thanks a lot for the adjustments!

Code changes -> ok
Test passed -> ok

-> Please merge to 4.4, create changelog and yaml and build

[troehmey/bug48138] 73300b974 Bug #48138: use current year in if condition
[troehmey/bug48138] 99419cd24 Bug #48138: small changes in regex
[troehmey/bug48138] cfa090b97 Bug #48138: undo if condition change
[troehmey/bug48138] b72a06df8 Bug #48138: fixed date issue
[troehmey/bug48138] b4c346d8f Bug #48138: fixed small code issues
[troehmey/bug48138] eb52c92ae Bug #48138: added accepted date formats
[troehmey/bug48138] f4237e79b Bug #48138: add unittest for birthday format
[troehmey/bug48138] a841af652 Bug #48138: birthday now accepted in german format
Comment 11 Toni Röhmeyer univentionstaff 2020-04-21 15:18:08 CEST
Fix with changelog and yaml were added to 4.4 with the following commits

commit 8d7c3624f80bb04441fd01996c9e2a21d89c4899
Bug #48138: added yaml

commit 2754d3b0c47819e4dd584e9d229e26c132d014d4
Bug #48138: added changelog entry

commit 962f1bc306605890dfdb0c76b356df30ab5093d0
Bug #48138: Merge branch 'troehmey/bug48138' into 4.4
Comment 12 Tobias Wenzel univentionstaff 2020-04-22 09:14:25 CEST
yaml -> correct version, entry ok
changelog -> ok
Comment 13 Tobias Wenzel univentionstaff 2020-04-22 09:53:59 CEST
Always add the build-information:

Successful build
Package: ucs-school-import
Version: 17.0.30A~4.4.0.202004211459
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 14 Tobias Wenzel univentionstaff 2020-07-30 15:12:11 CEST
UCS@school 4.4 v5 has been released (errata update to the release).

http://docs.software-univention.de/changelog-ucsschool-4.4v5-de.html#changelog:ucsschool:2020-04-27

If this error occurs again, please clone this bug.