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/core#1675 - Temporary regression fix for case add timeline #16926

Merged

Conversation

demeritcowboy
Copy link
Contributor

Overview

Detailed reproduction steps in ticket. When you add a timeline to a case using the Add Timeline dropdown near the middle of manage case it doesn't attach the last activity in the timeline to the case. The activity is created, but left on the non-case side.

Regression in 5.23 caused by DB package upgrade causing changes to how DB works with CRM_Core_Transaction, so the last transaction doesn't get committed.

This is a temp fix not the ideal fix, but I'm thinking for regression purposes at least against 5.24?

Technical Details

Lots of details in ticket.

Comments

The existing unit test for api case addtimeline didn't catch this because the transaction runs in an isolation level where the method of retrieving the resulting activities and checking them still succeeds even though they aren't committed to the db yet. I took a quick look at seeing if there was an easy way to deal with this in tests but I didn't see anything simple.

@civibot
Copy link

civibot bot commented Mar 30, 2020

(Standard links)

@civibot civibot bot added the 5.24 label Mar 30, 2020
@eileenmcnaughton
Copy link
Contributor

This is awful. I know it, you know it, the Postie's cousin knows it.

However I think that's covered in the code comments & associated issues & it makes sense to get a fix out quickly so I'm happy to go with 'this works' for now

@eileenmcnaughton eileenmcnaughton merged commit 4c43516 into civicrm:5.24 Mar 31, 2020
@demeritcowboy
Copy link
Contributor Author

Yes it is awful.

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