adobe-apiplatform / user-sync.py

Application for synchronizing Adobe customer directories via the User Management API
https://adobe-apiplatform.github.io/user-sync.py/en/user-manual/
MIT License
87 stars 67 forks source link

Update umapi.py - fixes #793 #794

Closed Luci2015 closed 2 years ago

Luci2015 commented 2 years ago

fixes #793

Summary

Testing Steps

Fixes #793

adorton-adobe commented 2 years ago

I've published an update to umapi-client that includes the fix (v2.20). Will you please update your branch with that version so I can do some testing?

adorton-adobe commented 2 years ago

This is going to take some time to test. Will you please also ensure that all tests pass? The build is breaking because at least one test is currently failing.

Luci2015 commented 2 years ago

Tests will continue to fail because your env still builds with umapi-client==2.19 and also some tests are set up to use the separate update statement inside the create call, when in fact that is no longer needed (check my latest test changes).

adorton-adobe commented 2 years ago

Your branch still has umapi-client==2.19 in the setup file. I updated my copy of your branch locally in order to test the updated client.

https://github.com/Luci2015/user-sync.py/blob/patch-1/setup.py#L54

And the test isn't failing because my environment is wrong. Once you update your branch's setup.py the test will still fail. The failing test is tests/test_umapi_engine.py::test_create_umapi_commands_for_directory_user which contains a check that ensures the UST is constructing the two-step create/update structure correctly.

>       assert vars(result) == vars(commands)
E       AssertionError: assert {'do_list': [...ratedID', ...} == {'do_list': [...ratedID', ...}
E         Omitting 3 identical items, use -vv to show
E         Differing items:
E         {'username': 'different@example.com'} != {'username': 'user1@example.com'}
E         {'do_list': [('create', {'country': 'US', 'email': 'user1@example.com', 'first_name': 'user1 First', 'last_name': 'user1 Last', ...})]} != {'do_list': [('create', {'country': 'US', 'email': 'user1@example.com', 'first_name': 'user1 First', 'last_name': 'user1 Last', ...}), ('update', {'email': 'user1@example.com', 'username': 'different@example.com'})]}
E         Full diff:
E           {
E            'do_list': [('create',
E                         {'country': 'US',
E                          'email': 'user1@example.com',
E                          'first_name': 'user1 First',
E                          'last_name': 'user1 Last',
E         -                'on_conflict': <IfAlreadyExistsOptions.ignoreIfAlreadyExists: 1>}),
E         +                'on_conflict': <IfAlreadyExistsOptions.ignoreIfAlreadyExists: 1>})],
E         ?                                                                                  +
E         -              ('update',
E         -               {'email': 'user1@example.com',
E         -                'username': 'different@example.com'})],
E            'domain': 'example.com',
E            'email': 'user1@example.com',
E            'identity_type': 'federatedID',
E         -  'username': 'user1@example.com',
E         ?               ^^  ^
E         +  'username': 'different@example.com',
E         ?               ^^^^  ^^^
E           }

You can probably just remove the part of the test that performs this check. If you've already fixed the test (and setup file) then you probably just need to push your changes.

Luci2015 commented 2 years ago

opened from patch-1

Luci2015 commented 2 years ago

done! I did not notice it was all building from patch-1 instead of v2. Now the tests, setup and umapi.py all come from patch-1 and tests pass.