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

SearchKit - Add CiviMail integration #22808

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Feb 20, 2022

Overview

Add a SearchKit task for "Email - schedule/send via CiviMail", similar to that in Advanced Search, etc.

Before

No convenient way to compose CiviMail directly from SearchKit.

After

Task will automatically create a hidden group (static, not smart group) and create a new mailing with it.

before

@civibot
Copy link

civibot bot commented Feb 20, 2022

(Standard links)

@civibot civibot bot added the master label Feb 20, 2022
@monishdeb
Copy link
Member

@colemanw I was testing the patch in the test build site http://core-22808-7ekai.test-1.civicrm.org:8001/ (admin / aDS90rFRafzZ)

And after building a searchkit display when I choose "Email - schedule/send via CiviMail" action, on the popup screen I am only allowed to fill the Mailing name nothing else. I cannot proceed from that point:
before

Am I missing something here?

@colemanw colemanw force-pushed the searchKitMailingTask branch from e74c2c6 to d2a39a8 Compare February 21, 2022 18:08
@colemanw
Copy link
Member Author

@monishdeb that's strange that you can't see the buttons at all.
When I test locally, the Submit & Cancel buttons are there, and they work.
When I test on the demo site you linked to, the buttons are there but they don't work (Submit is stuck as disabled)
But your screencast doesn't have any buttons.

There may be a dependency issue. I've rebased this PR so let's see if the new sandbox site works better.

@jaapjansma
Copy link
Contributor

Test this please

@monishdeb
Copy link
Member

I checked on my local and it works for me, heres screencast:
before

However there are few issues on the new test build site http://core-22808-7sbsp.test-3.civicrm.org:8001 but I think its not blocker to get this PR merge.

@monishdeb
Copy link
Member

@jaapjansma can you please ensure if this patch works for you too?

@jaapjansma
Copy link
Contributor

jaapjansma commented Feb 28, 2022

  • General standards
    • Explain (r-explain)
      • PASS : The goal/problem/solution have not been adequately explained in the PR. However it is quite user intuitive on what to do.
    • User impact (r-user)
      • PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
    • Documentation (r-doc)
      • ISSUE: We have checked the documentation and we believe that this new functionality should be documented as well as it makes sense for user to learn on how they can search for contacts and create a mailing from the search result.
    • Run it (r-run)
      • PASS: We created a new contact search with search kit and and then created an email from the results.
  • Developer standards
    • Technical impact (r-tech)
      • PASS: The change preserves compatibility with existing callers/code/downstream.
    • Code quality (r-code)
      • PASS: The functionality, purpose, and style of the code seems clear+sensible.
    • Maintainability (r-maint)
      • PASS: The change does not include tests however I believe the change is good enough to pass without a test.
    • Test results (r-test)
      • PASS: The test results are all-clear.

We believe that this PR is merge ready. However it needs attention when it comes to user documentation! @colemanw can you also add this functionality to the user documentation?

@jaapjansma jaapjansma added merge ready PR will be merged after a few days if there are no objections needs-documentation labels Feb 28, 2022
@colemanw colemanw merged commit 44572ea into civicrm:master Mar 10, 2022
@colemanw colemanw deleted the searchKitMailingTask branch March 10, 2022 13:17
@TomCrawshaw
Copy link

@colemanw Hi, can you say when this CiviMail integration is likely to go on general release, please?

@colemanw
Copy link
Member Author

@TomCrawshaw yes, it is part of version 5.49, which is the current beta, and will become the stable release in 3 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants