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] Minor Test clean up #13269

Merged
merged 3 commits into from
Dec 15, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor cleanup within the test classes

Before

Less clean

After

More clean

Technical Details

Uses a trait for permissions functions, some comments cleanup, one unused function removed

Comments

Have at it jenkins - oh when you are ready of course

This got separated out over a few recent commits (the first one seems to have somewhat
accidentally included the trait & then some others added using it. Am just pushing in this tidy up
@civibot
Copy link

civibot bot commented Dec 13, 2018

(Standard links)

* @param $where
*/
public function aclWhereOnlySecond($type, &$tables, &$whereTables, &$contactID, &$where) {
$where = " contact_a.id > 1";
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton Is this function unused? As I can't see a replacement in the trait?

* @param $where
*/
public function aclWhereOnlyOne($type, &$tables, &$whereTables, &$contactID, &$where) {
$where = " contact_a.id = " . $this->allowedContactId;
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton Is this function unused? As I can't see a replacement in the trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see above

@mattwire
Copy link
Contributor

@eileenmcnaughton So tests are passing. Assuming you meant to remove two unused functions aclWhereOnlyOne and aclWhereOnlySecond this is ok to merge - please confirm that's correct first?

@eileenmcnaughton
Copy link
Contributor Author

@mattwire yes, they are in the trait & the tests would have failed if there was a problem

@eileenmcnaughton eileenmcnaughton merged commit d9e8af2 into civicrm:master Dec 15, 2018
@eileenmcnaughton eileenmcnaughton deleted the acltesttrait branch December 15, 2018 02:07
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.

2 participants