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] Remove wrangling on activityType param #18558

Merged
merged 1 commit into from
Sep 23, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Removes an almost-redundant parameter & fixes the last place where it is still passed in

Before

Function called only from civi core & activityType almost always passed in (screenshot from slightly outdated civi-universe - in fact one of the 2 nulls is already fixed

Screen Shot 2020-09-22 at 5 03 16 PM

After

Screen Shot 2020-09-22 at 4 42 12 PM

Technical Details

There is only 1 remaining place that calls this function & does not specifiy activityType. This fixes
that place to pass in activityType and stops attempting to calculate activityType
based on in-function guess work

Comments

There is only 1 remaining place that calls this function & does not specifiy activityType. This fixes
that place to pass in activityType and stops attempting to calculate activityType
based on in-function guess work
@civibot
Copy link

civibot bot commented Sep 22, 2020

(Standard links)

@civibot civibot bot added the master label Sep 22, 2020
@eileenmcnaughton
Copy link
Contributor Author

test this please

@demeritcowboy
Copy link
Contributor

demeritcowboy commented Sep 23, 2020

I had looked at the recent related PR, and did a grep on core here and this looks ok to me.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 ok to merge based on ^^ ?

@seamuslee001 seamuslee001 merged commit 5ef1209 into civicrm:master Sep 23, 2020
@seamuslee001 seamuslee001 deleted the act_type branch September 23, 2020 02:47
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