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

Added conditional check so that it can be altered by hook #16499

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

pradpnayak
Copy link
Contributor

Overview

Added conditional check for printing blocks so that it can be easily altered by hooks if we need to hide certain blocks from printing.

@demeritcowboy

@civibot
Copy link

civibot bot commented Feb 10, 2020

(Standard links)

@civibot civibot bot added the master label Feb 10, 2020
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] Mostly PASS
      • It could mention that hook_civicrm_alterTemplateFile doesn't get called for this Report.tpl here, so you can't use that. You can customize CaseView.tpl though if you wanted to replace the Print Report link to use your own custom link. So two other approaches here then would be to make it either call hook_alterTemplateFile or make it easier to alter the print report link, but they might be more work.
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
  • Developer standards
    • [r-tech] PASS
      • A more general approach to customizing might be better long-term, but this change doesn't prevent that.
    • [r-code] PASS
      • It's mostly indentation changes. It's just adding in if statements that are similar to some that are already in the file for other sections.
    • [r-maint] PASS
    • [r-test] PASS

@pradpnayak
Copy link
Contributor Author

@demeritcowboy Yes i agree, ATM no Page/Form hooks are invoked to alter the smarty variables. But for now i am using casetypes() and/or optionValues() hooks to alter the smarty variables. But i am thinking to add support for hook_civicrm_alterContent ().

@yashodha
Copy link
Contributor

@demeritcowboy ready to merge or do you expect few changes in documentation etc?

@demeritcowboy
Copy link
Contributor

No there's nothing new to document. Seems fine to me.

@yashodha
Copy link
Contributor

@demeritcowboy ok, merging this PR as per your review.

@yashodha yashodha merged commit 1c7d6ef into civicrm:master Feb 11, 2020
@pradpnayak pradpnayak deleted the caseAuditReport branch March 23, 2020 01:05
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