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

REF Code cleanup for event configuration extracted from advancedevents extension #24418

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

mattwire
Copy link
Contributor

Overview

The advanced events currently overrides some core files. I'm slowly trying to remove that requirement.
This PR is a collection of a few small code cleanups that I made in the extension and should be in core.

Before

Less code cleanups

After

More code cleanups

Technical Details

Comments

@civibot
Copy link

civibot bot commented Aug 30, 2022

(Standard links)

@civibot civibot bot added the master label Aug 30, 2022
/**
* @return int
*/
protected function getEventId() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a "getter" function so we don't have to access class properties directly and it is clearer that $this->_id = event ID.

@@ -155,7 +161,7 @@ public function postProcess() {
CRM_Utils_System::redirect(CRM_Utils_System::url($url, $urlParams));
}
else {
CRM_Core_Error::statusBounce("Could not find Event ID");
CRM_Core_Error::statusBounce(ts('Could not find Event ID'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is shown to the user - make it translateable

@@ -94,22 +94,22 @@
<ul class="panel" id="panel_participants_{$row.id}">
{if $findParticipants.statusCounted}
<li>
<a class="action-item crm-hover-button" href="{crmURL p='civicrm/event/search'
<a title="{ts}Counted Participants{/ts}" class="action-item crm-hover-button" href="{crmURL p='civicrm/event/search'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Appears when you hover over the link

@demeritcowboy
Copy link
Contributor

This mergeable just a style issue in the comment header.

@demeritcowboy demeritcowboy merged commit 91997d6 into civicrm:master Aug 30, 2022
@mattwire mattwire deleted the advancedevents branch August 30, 2022 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants