Skip to content

Fix initUpdatedBy to check for updatedBy key#193

Open
Tim Körtgen (Timothylul) wants to merge 1 commit into
contentful:masterfrom
Timothylul:patch-1
Open

Fix initUpdatedBy to check for updatedBy key#193
Tim Körtgen (Timothylul) wants to merge 1 commit into
contentful:masterfrom
Timothylul:patch-1

Conversation

@Timothylul
Copy link
Copy Markdown

@Timothylul Tim Körtgen (Timothylul) commented May 12, 2026

I’m having an issue with the Management API because the updatedBy field is missing in the data array. From what I can see in the code, the check is currently based on whether createdBy is set, while the value being used is updatedBy. Was this intentional?

I tested my changes locally, and everything appears to be working correctly now after changing the check to use updatedBy instead.

Here is the exception that was thrown:

10:22:22 CRITICAL  [console] Error thrown while running command "REDACTED". Message: "Contentful\Core\Api\Link::__construct(): Argument #1 ($linkId) must be of type string, null given, called in /var/www/html/vendor/contentful/contentful-management/src/SystemProperties/Component/UpdatedByTrait.php on line 26" ["exception" => TypeError { …},"command" => "REDACTED","message" => "Contentful\Core\Api\Link::__construct(): Argument #1 ($linkId) must be of type string, null given, called in /var/www/html/vendor/contentful/contentful-management/src/SystemProperties/Component/UpdatedByTrait.php on line 26"]
 
In Link.php line 34:
  Contentful\Core\Api\Link::__construct(): Argument #1 ($linkId) must be of type string, null given, called in /var/www/html/vendor/contentful/contentful-management/src/SystemProperties/Component/UpdatedByTrait.php on line 26

Summary by Bito

The PR addresses an issue where the 'updatedBy' field is missing in the data array, causing a TypeError in the Link constructor. The fix changes the condition in initUpdatedBy to check for 'updatedBy' instead of 'createdBy', as tested locally.

Detailed Changes
  • Changes the isset check in initUpdatedBy from 'createdBy' to 'updatedBy' in UpdatedByTrait.php to match the data being used.

@Timothylul Tim Körtgen (Timothylul) requested a review from a team as a code owner May 12, 2026 13:14
@bito-code-review
Copy link
Copy Markdown

bito-code-review Bot commented May 12, 2026

Code Review Agent Run #d78267

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 2224102..2224102
    • src/SystemProperties/Component/UpdatedByTrait.php
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at jared.jolton@contentful.com.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review
Copy link
Copy Markdown

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted Summary
Bug Fix - Fix UpdatedBy initialization check
Corrected the condition in initUpdatedBy method to check for 'updatedBy' key instead of 'createdBy' to prevent null pointer errors when creating Link objects.

@bito-code-review
Copy link
Copy Markdown

Impact Analysis by Bito

Cross-Repository Impact Analysis
What Changed Impact of Change Suggested Review Actions
Fixed bug in UpdatedByTrait::initUpdatedBy() - changed isset($data['createdBy']) to isset($data['updatedBy']) - No cross-repo consumers found for initUpdatedBy: Search for initUpdatedBy returned zero hits across all repositories. This is a protected method within a PHP trait, consumed only internally by UpdatedTrait.
- No cross-repo consumers found for UpdatedByTrait: Search for UpdatedByTrait returned zero hits. This is an internal SDK trait used by the Contentful Management PHP library.
- Verify unit tests exist for UpdatedByTrait and UpdatedTrait
- Test that resources using UpdatedTrait correctly populate updatedBy field from API responses
- Confirm the fix aligns with CreatedByTrait pattern which correctly checks isset($data['createdBy'])
Code Paths Analyzed

Impact:
Bug fix in system properties trait. The initUpdatedBy method now correctly checks for 'updatedBy' key instead of erroneously checking 'createdBy'. This ensures the updatedBy Link object is properly initialized when the updatedBy data is present in the API response.

Flow:
API Response Data → Resource SystemProperties → UpdatedTrait::initUpdated() → UpdatedByTrait::initUpdatedBy() → Link object creation

Direct Changes (Diff Files):
• src/SystemProperties/Component/UpdatedByTrait.php [22-28] — Fixed isset check from 'createdBy' to 'updatedBy' in initUpdatedBy method

Repository Impact:
UpdatedTrait (uses UpdatedByTrait): UpdatedTrait composes UpdatedByTrait and calls initUpdatedBy() during initialization
Resources with system properties tracking update metadata: Any resource using UpdatedTrait will now correctly populate the updatedBy field

Cross-Repository Dependencies:
None.

Database/Caching Impact:
• None

API Contract Violations:
None.

Infrastructure Dependencies:
None.

Additional Insights:
Pattern consistency with CreatedByTrait: The fix aligns UpdatedByTrait with CreatedByTrait pattern - both now correctly check their respective field names (createdBy vs updatedBy)

Testing Recommendations

Frontend Impact:
None.

Service Integration:
• Verify that API responses containing updatedBy.sys data correctly populate the updatedBy Link object
• Test end-to-end resource retrieval to ensure updatedBy metadata is accessible via getUpdatedBy()

Data Serialization:
• Test jsonSerializeUpdatedBy() returns correct serialized format after the fix

Privacy Compliance:
None.

Backward Compatibility:
• This is a bug fix - previously the updatedBy field would never be populated due to the wrong isset check. After fix, resources will correctly expose updatedBy data where available.

OAuth Functionality:
• None

Reliability Testing:
• None

Additional Insights:
• Add unit test for UpdatedByTrait::initUpdatedBy() with mock data containing updatedBy field
• Compare test coverage with CreatedByTrait to ensure parity

Analysis based on known dependency patterns and edges. Actual impact may vary.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant