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

Enotice fix #22436

Merged
merged 1 commit into from
Jan 10, 2022
Merged

Enotice fix #22436

merged 1 commit into from
Jan 10, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 9, 2022

Overview

The form conditionally adds the export & close buttons to the template. The template does a check before adding the close button, and not before adding the export button. However, the conditional it DOES apply (checking status) is also applied by the form - so better to just check if it is applied.

Before

image

After

image

Technical Details

The buttons are not applied due to permissions - I think the demo user should have the ability to see these buttons & will do a PR to that effect

Comments

civicrm/civicrm-buildkit#666 adds the permissions

@civibot
Copy link

civibot bot commented Jan 9, 2022

(Standard links)

@civibot civibot bot added the master label Jan 9, 2022
@@ -75,17 +75,17 @@ CRM.$(function($) {
});
function assignRemove(recordID, op) {
var recordBAO = 'CRM_Batch_BAO_Batch';
if (op == 'assign' || op == 'remove') {
recordBAO = 'CRM_Batch_BAO_EntityBatch';
if (op === 'assign' || op === 'remove') {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton does === do the same thing in JS as in php?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 I think so since it was my IDE that wanted this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 seems to be working fine
image

Copy link
Member

Choose a reason for hiding this comment

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

Yes it does

@colemanw colemanw merged commit 649649a into civicrm:master Jan 10, 2022
@colemanw colemanw deleted the noticeme branch January 10, 2022 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants