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

dev/financial#163 Removal of contributionTypeID #19174

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

PR illustrates the remaining places where I propose to remove contributionTypeID. I will include this proposal in the dev-digest so this visualises is

Before

Hard to see what would be affected

After

PR makes the places clear

Technical Details

Depending on feedback this PR might be updated to actually remove the commented lines

Comments

@civibot
Copy link

civibot bot commented Dec 11, 2020

(Standard links)

@civibot civibot bot added the master label Dec 11, 2020
$value['contributionTypeID'] = $contribution->financial_type_id;
$value['receive_date'] = $contribution->receive_date;
$value['trxn_id'] = $contribution->trxn_id;
$value['contributionID'] = $contribution->id;
$value['contributionTypeID'] = $contribution->financial_type_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.

yeah we only need one of this & line 568

@eileenmcnaughton
Copy link
Contributor Author

I'm not going to worry about the test fails unless this seems hard to get agreement on - if agreed I'll change to remove, otherwise we should get this passing & committed for next round of this

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@mattwire
Copy link
Contributor

@eileenmcnaughton Yes definitely remove

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@eileenmcnaughton eileenmcnaughton changed the title dev/financial#163 Comment all the places where removal of contributionTypeID proposed dev/financial#163 Removal of contributionTypeID Dec 21, 2020
@eileenmcnaughton
Copy link
Contributor Author

This is now updated to remove rather than comment the relevant places

@eileenmcnaughton
Copy link
Contributor Author

@colemanw this should be good to merge now

@mattwire mattwire merged commit d830d3a into civicrm:master Dec 23, 2020
@eileenmcnaughton eileenmcnaughton deleted the conttype branch December 23, 2020 20:16
@eileenmcnaughton
Copy link
Contributor Author

First we take contributionTypeID then we take contributionMode

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