Skip to content

Fix refetchEntity call timing after Lead instance check #15051

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
Merged
merged 3 commits into from
Jun 26, 2025
Merged

Fix refetchEntity call timing after Lead instance check #15051

merged 3 commits into from
Jun 26, 2025

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented May 28, 2025 •
edited
Loading

Q A
Bug fix? (use the a.b branch) ✔️
New feature/enhancement? (use the a.x branch) ❌
Deprecations? ❌
BC breaks? (use the c.x branch) ❌
Automated tests included? ❌
Related user documentation PR URL N/A
Related developer documentation PR URL N/A
Issue(s) addressed N/A

Description

This PR fixes a potential issue where refetchEntity was being called on a null value when the lead is not found. The fix moves the refetchEntity call after the null check to ensure it's only called on valid Lead instances.

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Navigate to a page that uses the LeadDetailsTrait
  3. Ensure the code handles cases where a lead might not be found without throwing errors
  4. Verify that when a lead is found, the refetchEntity is properly called

@kuzmany
Fix: Move refetchEntity call after Lead instance check
f3c156c 
Prevents calling refetchEntity on null when lead is not found
@Copilot Copilot AI review requested due to automatic review settings May 28, 2025 19:36
Copilot
Copilot AI reviewed May 28, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that the refetchEntity method is only called on valid Lead instances by moving the call below the null/type check.

  • Prevents calling refetchEntity on a non-Lead (e.g., null) value.
  • Adjusts code layout to reflect the new call order.
Comments suppressed due to low confidence (1)

app/bundles/LeadBundle/Controller/LeadDetailsTrait.php:335

  • Add a unit test to verify behavior when getEntity returns null or a non-Lead instance, ensuring no errors are thrown and refetchEntity is not invoked.
if (!$lead instanceof Lead) {

@kuzmany @Copilot
Update app/bundles/LeadBundle/Controller/LeadDetailsTrait.php
e998f81 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@codecov Codecov
Copy link

codecov bot commented May 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 64.78%. Comparing base (cc73de5) to head (e17284a).
Report is 7 commits behind head on 6.0.

Files with missing lines Patch % Lines
...bundles/LeadBundle/Controller/LeadDetailsTrait.php 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                6.0   #15051   +/-   ##
=========================================
  Coverage     64.78%   64.78%           
  Complexity    34745    34745           
=========================================
  Files          2276     2276           
  Lines        103796   103796           
=========================================
  Hits          67248    67248           
  Misses        36548    36548
Files with missing lines Coverage Δ
...bundles/LeadBundle/Controller/LeadDetailsTrait.php 60.86% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kuzmany kuzmany added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels May 28, 2025
@kuzmany kuzmany added the T1 Low difficulty to fix (issue) or test (PR) label May 30, 2025
patrykgruszka
patrykgruszka approved these changes Jun 26, 2025
View reviewed changes
Copy link
Member

@patrykgruszka patrykgruszka left a comment

Choose a reason for hiding this comment

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

Code change looks fine 👍

@kuzmany kuzmany added this to the 6.0.3 milestone Jun 26, 2025
@kuzmany kuzmany removed the ready-to-test PR's that are ready to test label Jun 26, 2025
@kuzmany kuzmany merged commit b5179b1 into mautic:6.0 Jun 26, 2025
17 checks passed
@kuzmany kuzmany changed the title Fix: Move refetchEntity call after Lead instance check Fix refetchEntity call timing after Lead instance check Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@patrykgruszka patrykgruszka patrykgruszka approved these changes

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs T1 Low difficulty to fix (issue) or test (PR)
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
6.0.3
Development

Successfully merging this pull request may close these issues.
2 participants
@kuzmany @patrykgruszka