Skip to content

Use Case form for case email action #21688

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

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 1, 2021

Overview

Use Case specific email action form for case email action

Before

Handling for the case email action is overloaded onto the contact email action

After

it has it's own class

Technical Details

Comments

@civibot
Copy link

civibot bot commented Oct 1, 2021

(Standard links)

@eileenmcnaughton eileenmcnaughton changed the title Use Case form for case actions Use Case form for case email action Oct 1, 2021
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy I think this time I might have nailed moving the case stuff to it's own action form

@demeritcowboy
Copy link
Contributor

Ok will take a look.

@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 1, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 1, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 2, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 2, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 3, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
@demeritcowboy
Copy link
Contributor

I've been playing with this a bit. Found my notes about the url params: https://lab.civicrm.org/dev/core/-/issues/2463

Something else is now messing with the regular email form and it gives errors about count() for contactIds. Must be recent. Let's see...

@demeritcowboy
Copy link
Contributor

I think it's this one: #21680

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy argh - is it unchanged by this PR?

@demeritcowboy
Copy link
Contributor

Applying this PR doesn't change the error.

@demeritcowboy
Copy link
Contributor

I think the other one is simple, just checking closer.

@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Oct 3, 2021

@demeritcowboy FYI the 3 threads I'm working on are

  1. fix email to support missing token entities (membership, participant)
  2. fix pdf to support participant tokens (I think the open PR Add participant tokens to pdf task #21695 is the last one in this thread)
  3. fix metadata to load sanely & not advertise 'machine' tokens but still support them

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy so I'm guessing your other PR fixed ^^

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 5, 2021
This simplifies the code to 'listtokens' across most of the pdf & email classes.

The case tokens are a bit of a hold out as ideally we would have
one function on CRM_Case_Form_Task which email & pdf would use
but we are still getting to that point - see
civicrm#21688
@demeritcowboy
Copy link
Contributor

so I'm guessing your other PR fixed

Yes. Will try to finish this today.

@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy - do you agree that I probably need to conditionally group by contact id to extend tokens to participant / member for email? https://lab.civicrm.org/dev/core/-/issues/2862

@demeritcowboy
Copy link
Contributor

So this is really close but it does break the situation where you have Email configured in your case activity list dropdown on Manage Case, which is the other way to reach this task. In that situation there is no cid to start, but now it's somehow setting it to an array and it ends up with the reverse situation as the other problem earlier where now it wants null instead, and then you get an error. I think I'd be ok to merge but not to release it this way.

jenkins retest this please

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy do you know what the fix is for that?

@demeritcowboy
Copy link
Contributor

I haven't looked close but line 207 isn't expecting an empty array, and to avoid that normally it's null so it doesn't enter the if. But there's hopefully a clean fix.

@demeritcowboy
Copy link
Contributor

CiviCRM_API3_Exception: "invalid criteria for IN"

#0 \CRM\Contact\Form\Task\EmailTrait.php(207): civicrm_api3("Contact", "get", (Array:4))
#1 \CRM\Core\Form.php(650): CRM_Case_Form_Task_Email->buildQuickForm()
#2 \CRM\Core\QuickForm\Action\Display.php(76): CRM_Core_Form->buildForm()

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy ah yeah - that's in code that is munged into preProcess too - ideally we would extract $this->getContactDetails() & it would return already-calculated-value if one exists or calculate one

@eileenmcnaughton
Copy link
Contributor Author

Well merging this & we can track down that last little bit next

@eileenmcnaughton eileenmcnaughton merged commit 2184bfc into civicrm:master Oct 6, 2021
@eileenmcnaughton eileenmcnaughton deleted the case3 branch October 6, 2021 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants