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.

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

dev/core#2019 - Changing a case custom field from blank to something doesn't show properly what changed #19735

Merged

Conversation

demeritcowboy
Copy link
Contributor

@demeritcowboy demeritcowboy commented Mar 5, 2021

Overview

https://lab.civicrm.org/dev/core/-/issues/2019

  1. Create a custom field for cases.
  2. Create a case and leave the custom field blank.
  3. Change the custom field to something, let's say "Orange".
  4. Look in the resulting Change Custom Data activity on the case. It says it changed from "Orange => Orange", whereas you'd expect it to indicate the previous value was blank.

Before

Incorrect record of what changed.

After

Correct.

Technical Details

api customvalue.getdisplayvalue does an empty() check on the passed in value and then considers that to mean it should use the existing field value in the database instead, but it's already been updated and so contains the new value, so this then incorrectly treats '' and 0 which are legitimate prior values.

This api function doesn't appear to be used anywhere in universe, and was added in 5.19 for this function, so changing it is unlikely to affect anyone.

Comments

Has test.

This also includes a belated test for #19692 since the only thing extra needed to test that is to also run this test here in another locale, which it does.

@civibot
Copy link

civibot bot commented Mar 5, 2021

(Standard links)

@civibot civibot bot added the master label Mar 5, 2021
@eileenmcnaughton eileenmcnaughton merged commit 4515d9a into civicrm:master Mar 5, 2021
@demeritcowboy
Copy link
Contributor Author

Thanks!

@demeritcowboy demeritcowboy deleted the case-custom-origblank branch March 5, 2021 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants