Skip to content

🐛 Fix AttributeCaseLog serialization when field is hidden or read-only #8

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

Open
larhip wants to merge 1 commit into
base: master
Choose a base branch
from
Open

🐛 Fix AttributeCaseLog serialization when field is hidden or read-only #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions copy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,13 @@ function ReloadAndDisplay($oPage, $oObj, $sMessageId = '', $sMessage = '', $sSev
// value for attribute AttributeTagSet is an ormTagSet, it needs to be converted to json manually
$aCurrentValues[$sAttCode] = $oAttDef->GetJsonForWidget($oObjToClone->Get($sAttCode));
}
else if ($oAttDef instanceof AttributeCaseLog)
{
// AttributeCaseLog returns an ormCaseLog object which is not JSON-serializable via json_encode.
// GetEditValue() returns only the pending (new) entry as a plain string, which is what
// the WizardHelper widget expects as current value.
$aCurrentValues[$sAttCode] = $oAttDef->GetEditValue($oObjToClone->Get($sAttCode));
Copy link
Copy Markdown
Collaborator

@Hipska Hipska Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$aCurrentValues[$sAttCode] = $oAttDef->GetEditValue($oObjToClone->Get($sAttCode));
$aCurrentValues[$sAttCode] = $oObjToClone->GetEditValue($sAttCode);

Maybe this should just be done like this for the else branch, so this addition is not needed? (and the one for AttributeDateTime could then be removed as well)

Copy link
Copy Markdown
Collaborator Author

@larhip larhip Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Thomas! Using GetEditValue() in the else branch is indeed cleaner.
However, it might open a separate can of worms: AttributeExternalKey::IsScalar() returns true, so hidden/readonly external key fields pass the guard and land in $aCurrentValues. DBObject::GetEditValue() returns the friendly name for external keys rather than the integer ID — which ajax.render.php and the WizardHelper expect. Switching the else branch might silently break hidden external key fields.

To keep this PR focused on the CaseLog issue, I'd prefer to go with the minimal fix for now. The broader cleanup (consolidating to GetEditValue() + an explicit AttributeExternalKey guard for the ID) could be a follow-up if desired.

What do you think?

}
else
{
$aCurrentValues[$sAttCode] = $oObjToClone->Get($sAttCode);
Expand Down