-
-
Notifications
You must be signed in to change notification settings - Fork 825
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.
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
CiviEvent - Error registering participants via search task #19125
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
6161ec5
Fix Fatal Error on trying to use the Register contacts to an event Ad…
seamuslee001 1db1f8e
Additional fix
eileenmcnaughton df80fba
Event Registration Task - Propagate different qfKey & action for AJAX…
totten 2497350
CRM_Event_Form_Task_Register - Fix error "getFieldValue failed" (tent…
totten File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of this TRUE has caused the 500 console error that I added it to avoid to come back. The other symptom is the fees not loading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's weird. I was seeing the opposite - even with
ignoreKey=TRUE
here (PR-branch without df80fba), the AJAX call replied 500 and the fees subform didn't render. With df80fba (with a valid key -- and withignoreKey=FALSE
), the AJAX call and the subform worked.I wonder if there might be some subtle difference in the pageflows or use-cases that we're trying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@totten maybe - I have a good idea of a better fix - but we need to get 'something' out - to be honest I can't understand how the key would be rejected if ignoreKey is false
I am doing a basic search & then selecting some contacts & choosing register participants
I did test that on the pr test site & it failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, my procedure is very similar, ie
@eileenmcnaughton I can reproduce the error that appears on the test site. (Incidentally, I'm doing this on a different workstaiton/build.) Notably, in both test site and local, the 500 error is not related to qfKey. It's about "getFieldValue failed" (which curiously arose before but had seemed transitory). Here's a backtrace.
The thing is... to my reading, it's basically saying that the event-registration form's preProcess() is applying a mismatched security check. (It's trying to use a
contactId
andCRM_Contact_Page_View::checkUserPermission()
-- but in the context of a search-task, it doesn't make sense to use a singularcontactId
for assessing permissions on a bulk action.)I've pushed up another patch which fixes this symptom for me, but it definitely needs some more eyes to see if it's faithful to contract/permissioning of this route/controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@totten given that
I'm inclined to merge what that person merged & work on the right fix in master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The report on 5.32 PR actually pointed to the same problem raised here (just a diff part of the manifestation -- the HTTP 500 error you noted above is the same "getFieldValue required" error he observed) - I think they just didn't realize that the screen was misbehaving (because it's a misbehavior of omission; because the immediate error-message is hidden in AJAX console; and the UI-level error message is delayed). So the revision they tested wasn't working.
Added some more comments over there to try to distinguish 3 issues floating around here.
To my testing, the current revision of this branch addresses all 3.