Contribution Pages: alias userID to contactID, fixes org renewal with checksum and mid #22951
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
… checksum and mid
|
(Standard links)
|
|
jenkins, test this please |
eileenmcnaughton
added a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 6, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
eileenmcnaughton
added a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 6, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
|
@eileenmcnaughton oh.. thanks! I had a quick look, enough to understand that this PR can be closed, but will do more testing. |
mlutfy
pushed a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 12, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
mlutfy
pushed a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 12, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
mlutfy
pushed a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 12, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
eileenmcnaughton
added a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Aug 12, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
larssandergreen
pushed a commit
to larssandergreen/civicrm-core
that referenced
this pull request
Aug 22, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
larssandergreen
pushed a commit
to larssandergreen/civicrm-core
that referenced
this pull request
Sep 6, 2023
This is a reviewer response to civicrm#22951 In that PR it seeks to set UserID to something derived from a function that is doing a lot of work. I dug into that function which someone (ahem) wrote many years ago and teased out some of the underlying chunks & cleaned up the variable names for clarity along with a doc block cleanup. I decided that rather than set _userID in contributionBase we should remove it (it is only referred to in 4 places) and call the more specific function at each usage. Otherwise we change the meaning of userID that is used somewhat differently later. Also I rather prefer functions that will perform the same whenever called over relying on a variable having been set correctly the first time
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
Organisational membership renewal is a bit of a challenge. One can send a Scheduled Reminder to the individual related to an organisation, with the checksum/cid of the individual, and if their relationship to the org allows it, they can view the organisation and renew on-behalf.
However, the contribution page does not show any information about the membership being renewed.
Form have a way around that: we can pass the little-known
midURL argument (membership ID), but it only works if the user is logged-in (and not with a checksum). This membershiprenewallink extension by Jaap is pretty useful for that.Looking at the code, there is a
userIDandcontactIDvar, with a mysterious comment about it:I can't see a good reason to distinguish between a logged-in user, and a checksum user.
Before
Passing the
midas an URL argument will cause a fatal error, because it checks against the$this->_contactID, which is NULL if we used a checksum.After
Using the
midin the URL, we can now select the membership to renew, and everything works as expected: