Skip to content
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

API changelog eventbrite. #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

harshal6
Copy link

Changed the /webhook to /organizations/{organization_id}/webhooks

// Omitting this line causes the setting to be omitted from the Settings form:
// 'quick_form_type' => 'Element',
// adding it back to implement the new api.
'quick_form_type' => 'Element',
Copy link
Owner

Choose a reason for hiding this comment

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

Hi @harshal6 This has the effect of making the 'Eventbrite API Organization ID' setting editable by the site admin, but I'm not sure this is advisable. The extension operates under the assumption that the given 'Eventbrite Personal OAuth Token' is only in use for one organization, and it then sets the 'Eventbrite API Organization ID' setting automatically. Have you found some need to edit this value manually?

Copy link
Author

Choose a reason for hiding this comment

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

There is no specific need, initially I did not realise we had organization which was autopopulated so made it editable, let me update the code and make it non editable. @twomice thanks

Copy link
Author

@harshal6 harshal6 left a comment

Choose a reason for hiding this comment

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

updated the PR and made the organization id uneditable.

// Omitting this line causes the setting to be omitted from the Settings form:
// 'quick_form_type' => 'Element',
// adding it back to implement the new api.
'quick_form_type' => 'Element',
Copy link
Author

Choose a reason for hiding this comment

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

There is no specific need, initially I did not realise we had organization which was autopopulated so made it editable, let me update the code and make it non editable. @twomice thanks

@harshal6 harshal6 requested a review from twomice October 20, 2020 12:25
@twomice
Copy link
Owner

twomice commented Oct 20, 2020

Thanks @harshal6 . Tell me please, what steps have you taken to verify that this PR has the desired effect? Specifically, it would be very helpful if you have a moment to answer these basic questions:

Before:
We know from the OP #22 that the current codebase generates an error from EventBrite, "403: NOT_AUTHORIZED: This user is not able to use legacy user endpoints, please use the organization equivalent," when performing certain steps. I think this is clear enough, but if there's any additional information please add it here.

After:
What changed? (For example, did you perform the steps from "Before" and observe that there's no such error? Did you verify that the webhooks are actually created in your EventBrite account? Etc.)

Technical Details:
If the PR involves technical details/changes/considerations which would not be manifest to a casual developer skimming the above sections, please describe the details here.

Comments:
Anything else you would like the reviewer to note

Whatever information you can provide for the above sections will be very helpful to us in performing the review before merging. We can of course do all this ourselves here, but the above information from you will definitely speed things along. Thanks!

@harshal6
Copy link
Author

harshal6 commented Oct 20, 2020

Before
Error establishing webhook configuration via Eventbrite API. Eventbrite said: 403: NOT_AUTHORIZED: This user is not able to use legacy user endpoints, please use the organization equivalent.
After

  • The error disappeared on the settings form.
  • I tested and I can see the request payload sends data successfully to CiviCRM
  • I checked the API Explorer and I can see the EventbriteQueue record created with the same webhook id as in the payload.

Technical Requirements
N/A

Comments
What I could not test or know on how the events on civicrm and eventbrite are synced. I do see it in Eventbrite Integration Settings: admin/eventbrite/manage/events?action=browse&reset=1

Basically I did not test this
"The sync happens in near-real-time based on EB webhooks, so EB tickets will only be synced into CiviCRM if they are purchased (or modified) after you set up the appropriate configuration for event and ticket types within this extension."
@twomice for reviewing.

@axelady
Copy link

axelady commented Dec 23, 2021

FYI. I found this solution and it worked for me. Got it running for a live event.

I did encounter some problems with the integration that are probably not related to this change.

  1. Persons who chose to send in a check later: Their transaction was pushed to civi in a completed state, not pending.
  2. This may have had to do with set-up. We had several sessions of the same event - all set to register for a single event in civi. If a person selected more than one session, the amount pushed was just for one session, not the total that they actually paid.
    These two issues caused us to have to manually trace and correct errors, which kind of defeats the purpose of having an automatic integration in the first place.

@twomice
Copy link
Owner

twomice commented Dec 27, 2021

Thanks for the confirmation @axelady . I'm ready to merge this but I think the code needs some simple fixes to meet CiviCRM Coding Standard. I'm trying to make time for that soon.

Regarding your other two points: If you could create separate issues for these 2 problems it would help us to track them. I can tell you that at the moment this extension is not being used by our clients, so it's hard for us to make time to address these issues, but we'd at least like to a) make people aware of the limitations and b) review and accept PRs to address those issues.

@lcarterBOT
Copy link

Not to hijack this thread but I'd be greatly curious to know how @axelady got the integration to work. We made the changes noted above but we are still seeing the 403 error; curiously, we can pull in the ticket options from the event (which suggests it's correctly connecting to some extent) but cannot pull in the current registrants from EB.

@axelady
Copy link

axelady commented Feb 7, 2022

@lcarterBOT I grabbed some screenshots and tried to document here: https://www.dropbox.com/s/2vcf0s6f36r9tbq/SETUP.docx?dl=0
This was after editting the code with the changes in this PR.

@lcarterBOT
Copy link

@axelady Thank you very much - this is really great and I appreciate your taking the time to put this together. One additional question: is the scheduled job correctly importing participants and registrations back into Civi for you? That's the remaining piece for us - we can see the event and connect to the ticket options, but no registrations are being brought back in.

@twomice
Copy link
Owner

twomice commented Feb 7, 2022

@axelady Thanks very much for providing that document. I'd like to attach the document directly to this PR (so that it's still available if and when it's no longer on Dropbox), and possibly integrate its content into the extension docs. Are you agreeable to that?

@axelady
Copy link

axelady commented Feb 7, 2022

@twomice
SETUP.docx

@lcarterBOT Yes, it imported participants and registrations, with the errors that I mentioned above (filed issues for each). We also use the civiquickbooks extension and the registration payments pushed nicely into quickbooks from there.

I don't recall having done anything with the scheduled job, here's a screenshot of those settings:
image
I'm curious about the runInNonProductionEnvironment=TRUE if that makes a difference.

Also, my cron is set at every 10 mins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants