Ui + test coverage. See parent.

Steps:

  1. Change reaction-rule config to save the event as: events => { event_name => { array of event settings } }. This needs adaptions in tests, config entity and in the config schema. #2666200: Change reaction-rule config to save the event with settings
  2. Change current UI code to incorporate the event form and write event form for bundle plugin #3106987: Add event bundle selection to "Add a reaction rule" form Assigned to: TR
  3. Test coverage to edit an event in the UI #2676514: UI test coverage for adding an event with per-bundle entities

Pull request

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

a.milkovsky created an issue. See original summary.

fago’s picture

Steps:
1. Change reaction-rule config to save the event as: events => { event_name => { array of event settings } }. This needs adaptions in tests, config entity and in the config schema. (
2. Change current UI code to incorporate the event form and write event form for bundle plugin
3. Test coverage to edit an event in the UI

a.milkovsky’s picture

Issue summary: View changes
a.milkovsky’s picture

Issue summary: View changes

Linked PR. Work in progress.

a.milkovsky’s picture

Assigned: a.milkovsky » Unassigned
Issue summary: View changes
Status: Active » Needs review

As the form has an ajax request, we can not do Ui tests right now, javascript is unsupported in Ui tests for now. Test will be implemented in a separate task.
Else is ready to review.

fago’s picture

There are some issues left, but as this covers the API change for multiple events I'll make sure this is committable and push the changes + open a follow-up for the rest.

fago’s picture

Status: Needs review » Needs work

ok, I fixed conflicts and did some basic fixes - still there are test fails we need to resolve before being able to merge this. My changes are at https://github.com/fago/rules/pull/430 and https://travis-ci.org/fago/rules/jobs/114230913

fago’s picture

Priority: Normal » Critical
a.milkovsky’s picture

Issue summary: View changes

Updated PR link

a.milkovsky’s picture

@fago,
When I run 'Drupal\Tests\rules\Functional\ConfigureAndExecuteTest::testConfigureAndExecute'
I get the "Warning: mkdir(): Permission denied in /.../vcs/web/core/lib/Drupal/Component/PhpStorage/FileStorage.php on line 170"
How could I get rid of this error?

klausi’s picture

make sure to fix the file permissions in your drupal installation. the web server user must have write access to the sites directory where it creates the test site directory.

a.milkovsky’s picture

Issue summary: View changes
dasjo’s picture

Do we still need this one? Maybe someone could pick it uo again?

TR’s picture

I'm posting the patch here so the testbot can look at it and we can continue development.

This is https://patch-diff.githubusercontent.com/raw/fago/rules/pull/472.patch which was last modified on 7 March 2016.

TR’s picture

Assigned: Unassigned » TR

As far as I can tell, much of the code in the PR and the above patch was already committed as part of #2658842: Add support for Rules event handlers and per-bundle entities on 25 Feb 2016. Sorting out what remains to be completed is going to be a pain.

This issue largely overlaps with the two child issues and greatly overlaps with the already-closed parent issue. I'm inclined to just identify the unfinished tasks from those three and this issue and move them all into a new meta.

I already have a lot of this work completed on my local, so I will create the new meta when I'm ready to post that patch.

TR’s picture

Component: Rules Core » User Interface
Priority: Critical » Major
Issue summary: View changes

Made changes to the issue summary. I'm still sorting out what has been done and what still needs to be done here.

TR’s picture

Issue summary: View changes
TR’s picture

Issue summary: View changes

Removed task 3 from the issue summary. See my reasoning in #2676514: UI test coverage for adding an event with per-bundle entities. Specifically, Javascript testing IS possible in core Drupal at this time so there is no need to have a separate issue for the tests - tests should be included with any patch in this issue.

TR’s picture

Issue summary: View changes
Status: Needs work » Needs review

Added link to issue for task 2.

TR’s picture

Status: Needs review » Fixed

All steps in the original post have been addressed. Nothing left to be done.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.