Add setEntityId() to entityForm #16020
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
|
@mattwire test failure |
|
Hey @seamuslee001 is this failing because of the PropertyBag deprecated warnings? |
|
@mattwire no |
e9f6567 to
b1b2df1
Compare
CRM/Core/Form/EntityFormTrait.php
Outdated
| * | ||
| * @var int | ||
| */ | ||
| protected $_id; |
There was a problem hiding this comment.
@mattwire is there a reason why your making this protected rather than public?
b1b2df1 to
2602c9f
Compare
CRM/Member/Form/MembershipConfig.php
Outdated
| @@ -28,7 +28,7 @@ class CRM_Member_Form_MembershipConfig extends CRM_Core_Form { | |||
| * | |||
| * @var int | |||
| */ | |||
| public $_id; | |||
| protected $_id; | |||
There was a problem hiding this comment.
@mattwire can you revert this change?
aede38b to
c067823
Compare
|
@mattwire as your changing the visibility of $_id in CRM_Admin_Form then you need to make these changes as well diff --git a/CRM/Admin/Form/Job.php b/CRM/Admin/Form/Job.php
index 8a8679b5b4..3009a5b730 100644
--- a/CRM/Admin/Form/Job.php
+++ b/CRM/Admin/Form/Job.php
@@ -19,7 +19,7 @@
* Class for configuring jobs.
*/
class CRM_Admin_Form_Job extends CRM_Admin_Form {
- protected $_id = NULL;
+ public $_id = NULL;
public function preProcess() {
diff --git a/CRM/Admin/Form/LabelFormats.php b/CRM/Admin/Form/LabelFormats.php
index 1713fd910b..9c60fe8de7 100644
--- a/CRM/Admin/Form/LabelFormats.php
+++ b/CRM/Admin/Form/LabelFormats.php
@@ -41,7 +41,7 @@ class CRM_Admin_Form_LabelFormats extends CRM_Admin_Form {
* Label Format ID.
* @var int
*/
- protected $_id = NULL;
+ public $_id = NULL;
/**
* Group name, label format or name badge
diff --git a/CRM/Admin/Form/PaymentProcessorType.php b/CRM/Admin/Form/PaymentProcessorType.php
index 571aa957ac..89adf8ac7b 100644
--- a/CRM/Admin/Form/PaymentProcessorType.php
+++ b/CRM/Admin/Form/PaymentProcessorType.php
@@ -19,7 +19,7 @@
* This class generates form components for Location Type.
*/
class CRM_Admin_Form_PaymentProcessorType extends CRM_Admin_Form {
- protected $_id = NULL;
+ public $_id = NULL;
protected $_fields = NULL;
diff --git a/CRM/Admin/Form/PdfFormats.php b/CRM/Admin/Form/PdfFormats.php
index 8588c55516..0ec9042d58 100644
--- a/CRM/Admin/Form/PdfFormats.php
+++ b/CRM/Admin/Form/PdfFormats.php
@@ -41,7 +41,7 @@ class CRM_Admin_Form_PdfFormats extends CRM_Admin_Form {
* PDF Page Format ID.
* @var int
*/
- protected $_id = NULL;
+ public $_id = NULL;
/**
* Build the form object.
diff --git a/CRM/Admin/Form/ScheduleReminders.php b/CRM/Admin/Form/ScheduleReminders.php
index a845b5c644..e0fd77de3c 100644
--- a/CRM/Admin/Form/ScheduleReminders.php
+++ b/CRM/Admin/Form/ScheduleReminders.php
@@ -24,7 +24,7 @@ class CRM_Admin_Form_ScheduleReminders extends CRM_Admin_Form {
* Scheduled Reminder ID.
* @var int
*/
- protected $_id = NULL;
+ public $_id = NULL;
public $_freqUnits;
|
2bd7e0a to
9f92c58
Compare
9f92c58 to
b1c7169
Compare
|
@mattwire same test is still failing |
b1c7169 to
e57fc8c
Compare
|
@seamuslee001 Think we're finally there with this one! |
|
I'm just removing merge ready because that doesn't mean the way you are using it - it you give it to another person's Pr when you are ready to merge it but just want to give it few days cool off inn case someone else has a comment. Merge on pass is when you review another person's PR & it can be merged as soon as tests pass. There are 2 things I'd want to check before merging this
|
|
Oh - the latter point -we have getEntityID & this adds setEntityId - I think it has to be caps then |
Believe me I tried! Unfortunately the membership classes access the entity ID which (currently) require it to be public as they access the id property directly.
I'm confused. We have |
|
Ah - I based that on reading the PR not the code - PR comment is wrong it's entityId as you say. OK - I'm happy to merge then |
|
Thanks @eileenmcnaughton if I get chance I'll try and follow up sometime with some work on the membership classes to use this function and then we could probably switch back to a protected property. |
|
@mattwire yeah that would be good - they just need to call the sets & gets |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Add a function to set the entity ID - we can already get it via
getEntityID()but not set.Before
No function to set entity ID on the form.
After
Function to set the entity ID on the form.
Technical Details
Comments
I'm looking at making some improvements / conversion to contributionview form and this is a pre-requisite to doing it properly.