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.

Sign up for GitHub

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

Jump to bottom

Add defaults for APIv4 Case create #20306

Merged
merged 2 commits into from
May 25, 2021
Merged

Add defaults for APIv4 Case create #20306

merged 2 commits into from
May 25, 2021

Conversation

aydun
Copy link
Contributor

@aydun aydun commented May 14, 2021

Overview

Add defaults for APIv4 Case.create:

  • default Case Creator = current user
  • default Case start date = now

Before

No defaults for Case Creator and Case Start Date

After

Sensible defaults for Case Creator and Case Start Date

Technical Details

Comments

@aydun
Add defaults for APIv4 Case
ec039b8 
 - default Case Creator = current user
 - default Case start date = now
@civibot
Copy link

civibot bot commented May 14, 2021

(Standard links)

@civibot civibot bot added the master label May 14, 2021
@colemanw
Copy link
Member

Agree this looks sensible.

@eileenmcnaughton
Copy link
Contributor

It seems Coleman was more impressed than jenkins

api\v4\Entity\ConformanceTest::testConformance with data set "Case" ('Case')
PEAR_Exception: DB Error: unknown error

/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Core/Error.php:943
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php:922
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/db/DB.php:998
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php:575
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php:223
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/db/DB/common.php:1928
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/db/DB/mysqli.php:936
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/db/DB/mysqli.php:406
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/vendor/pear/db/DB/common.php:1234
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/packages/DB/DataObject.php:2696
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/packages/DB/DataObject.php:1245
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Core/DAO.php:639
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Case/XMLProcessor/Process.php:248
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Case/XMLProcessor/Process.php:231
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Case/XMLProcessor/Process.php:96
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/CRM/Case/XMLProcessor/Process.php:41
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php:77
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/Api4/Action/CiviCase/CiviCaseSaveTrait.php:38
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/Api4/Generic/DAOCreateAction.php:41
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/Api4/Provider/ActionObjectProvider.php:68
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/API/Kernel.php:149
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/Civi/Api4/Generic/AbstractAction.php:239
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Entity/ConformanceTest.php:206
/home/jenkins/bknix-dfl/build/core-20306-252nq/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Entity/ConformanceTest.php:139
/home/jenkins/bknix-dfl/extern/phpunit7/phpunit7.phar:615

@colemanw
Copy link
Member

I don't think the conformance test creates a logged-in user.

@eileenmcnaughton
Copy link
Contributor

@colemanw so it IS possible via the cli for there to be no logged in user so this should not db-error without one

@eileenmcnaughton
Copy link
Contributor

@colemanw @aydun so it needs to handle no-one being logged on sensibly - in which case there would be no default

@colemanw
Copy link
Member

I think the API works fine, it's just a problem with the test. The conformance test assumes that if a field has a default value, it shouldn't fill that field. But if the default is 'user_contact_id' then it actually does need to fill it because there is no logged-in user.

@colemanw
APIv4 - Fix Case conformance test
6e81498 
Now that creator_id has a default value it can also be marked as required.
Updating TestCreationParameterProvider to supply a FK before the default value
fixes the problem with user_contact_id in a test with no logged-in user.
@colemanw
Copy link
Member

I've pushed up a fix to the test.

@eileenmcnaughton eileenmcnaughton merged commit 69d0d5f into civicrm:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
master merge on pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aydun @colemanw @eileenmcnaughton