Skip to content

Fix #14562: Resolve error while creating company custom field #14566

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 1 commit into from
Feb 10, 2025
Merged

Fix #14562: Resolve error while creating company custom field #14566

merged 1 commit into from
Feb 10, 2025

Conversation

shinde-rahul
Copy link
Contributor

@shinde-rahul shinde-rahul commented Feb 7, 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 mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #14562

Description

When creating a custom field for companies, there is an error, and the field is not being created.

Please see this

image

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Go to Custom Fields (/s/contacts/fields).
  3. Click the "New" button.
  4. Enter the required details and select Company as the Object.
  5. Save the field; it should be created successfully.

For existing setup,

  • Run migration.

@codecov Codecov
Copy link

codecov bot commented Feb 7, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.07%. Comparing base (7f95afd) to head (33add80).
Report is 56 commits behind head on 6.x.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                6.x   #14566      +/-   ##
============================================
+ Coverage     64.04%   64.07%   +0.03%     
- Complexity    34670    34677       +7     
============================================
  Files          2287     2287              
  Lines        103767   103789      +22     
============================================
+ Hits          66453    66503      +50     
+ Misses        37314    37286      -28
Files with missing lines Coverage Δ
app/bundles/LeadBundle/Entity/LeadField.php 96.53% <100.00%> (+0.04%) ⬆️

... and 13 files with indirect coverage changes

@shinde-rahul shinde-rahul requested a review from escopecz February 7, 2025 04:57
@shinde-rahul shinde-rahul added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging custom-fields Anything related to custom fields labels Feb 7, 2025
@shinde-rahul shinde-rahul mentioned this pull request Feb 7, 2025
Closed
kuzmany
kuzmany approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Yes, I faced this issue and this figure out it

abhisekmazumdar
abhisekmazumdar approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@abhisekmazumdar abhisekmazumdar left a comment

Choose a reason for hiding this comment

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

I see this is an issue in version 6.x. The changes fix the problem. Looks good to me.

@kuzmany
Copy link
Member

kuzmany commented Feb 7, 2025

This change requires migration.

@shinde-rahul shinde-rahul added code-review-passed PRs which have passed code review ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people. and removed code-review-needed PR's that require a code review before merging ready-to-test PR's that are ready to test labels Feb 7, 2025
@escopecz
Copy link
Member

escopecz commented Feb 7, 2025

@kuzmany I can see a migration. Does it require another one?

@shinde-rahul
Copy link
Contributor Author

I have updated the description to execute the migration for the existing setup. He (kuzmany) might be suggesting this.

@escopecz
Copy link
Member

@kuzmany can you please explain what you meant by this?

This change requires migration.

I'd like to clear that up before merge.

@kuzmany
Copy link
Member

kuzmany commented Feb 10, 2025

@escopecz it change DB schema for isShortVisible column, then it should be applied as migration to avoid too much diff in schema udpate

@escopecz
Copy link
Member

@kuzmany I'm confused. This PR has a migration in it. Please check the diff. Is there any problem with this migration?

@kuzmany
Copy link
Member

kuzmany commented Feb 10, 2025

@escopecz ah, my mistake. Sorry. Good to go 👍

@escopecz escopecz merged commit 1ef850c into mautic:6.x Feb 10, 2025
17 checks passed
@escopecz escopecz added this to the 6.0.0-beta milestone Feb 10, 2025
@RCheesley RCheesley modified the milestones: 6.0.0-beta, 6.0.0-beta2 Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kuzmany kuzmany kuzmany approved these changes

@escopecz escopecz Awaiting requested review from escopecz

@andersonjeccel andersonjeccel Awaiting requested review from andersonjeccel

+1 more reviewer

@abhisekmazumdar abhisekmazumdar abhisekmazumdar approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs code-review-passed PRs which have passed code review custom-fields Anything related to custom fields ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people.
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
6.0.0-beta2
Development

Successfully merging this pull request may close these issues.

There is an error while creating a custom Field for the company
5 participants
@shinde-rahul @kuzmany @escopecz @abhisekmazumdar @RCheesley