Skip to content

[Bug]: Omit Mandatory Check on field with Default Value when creating a new DataObject#15812

Merged
dvesh3 merged 5 commits into11.0from
mandatory-default-add-new
Aug 29, 2023
Merged

[Bug]: Omit Mandatory Check on field with Default Value when creating a new DataObject#15812
dvesh3 merged 5 commits into11.0from
mandatory-default-add-new

Conversation

@kingjia90
Copy link
Copy Markdown
Contributor

@kingjia90 kingjia90 commented Aug 24, 2023

Changes in this pull request

Resolves #15311

Additional info

WHAT

🤖 Generated by Copilot at 5c02d95

Fix mandatory check for fields with default values in DataObject/Concrete.php. This allows saving new objects with empty mandatory fields that have a default value set in the class definition.

🤖 Generated by Copilot at 5c02d95

default values
skip the mandatory check
autumn leaves falling

HOW

🤖 Generated by Copilot at 5c02d95

  • Skip mandatory check on fields with default value when adding new object (link)

@kingjia90 kingjia90 added the Bug label Aug 24, 2023
@kingjia90 kingjia90 added this to the 11.0.8 milestone Aug 24, 2023
@kingjia90 kingjia90 linked an issue Aug 24, 2023 that may be closed by this pull request
@github-actions
Copy link
Copy Markdown

Review Checklist

  • Target branch (11.0 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@kingjia90 kingjia90 force-pushed the mandatory-default-add-new branch from f803eb4 to b5faed4 Compare August 24, 2023 09:49
@kingjia90 kingjia90 force-pushed the mandatory-default-add-new branch from 90f737b to e9c2253 Compare August 24, 2023 10:29
@kingjia90
Copy link
Copy Markdown
Contributor Author

kingjia90 commented Aug 24, 2023

Notes about Codeception Tests: in https://github.com/pimcore/pimcore/actions/runs/5962567143/job/16173995193#step:11:3166 (commit e9c2253) the error there is after the exception get caught and string contain asserted true and that's positive as it was intentionally to check if save was unsuccessfull and the wanted exception got caught, 3e95403 (ndr it's passed, only sonar cloud crashed causing false negative) meaning save() in try block was successfull and reached the assertEquals of the default value.
Then cleaned up as these were "inverted tests"(expecting to fail but with assert in the catch block), but this PR fixes the initial problem and can't intentionally make it crash, however it would crash in future if the problem resurfaces/regresses, eg. by commenting the fix it returns
image

@dvesh3 dvesh3 self-assigned this Aug 28, 2023
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: the default value is not used for mandatory fields

2 participants