Problem/Motivation

When this issue #2648802: Deleting a choice should delete the votes over it in the database will be fixed, deleting a choice will delete all the votes over it, so we must alert the user in case this choice has votes on it (or maybe just put a general alert, to be discussed).

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

edurenye created an issue. See original summary.

Bambell’s picture

Assigned: Unassigned » Bambell
edurenye’s picture

Assigned: Bambell » edurenye
Status: Active » Needs review
FileSize
22.85 KB

Finally I found how to do that.

This is how it looks like:

edurenye’s picture

Missed the patch, sorry.

edurenye’s picture

Status: Needs review » Postponed

I think we need JS tests for that, so postponed until #2610150: Readd tests for Ajax Poll Vote is in.

Berdir’s picture

Status: Postponed » Needs work

That is in, lets add test for this now.

  1. +++ b/js/poll.js
    @@ -0,0 +1,20 @@
    +      $(Drupal.theme('pollChoiceDeleteWaring')).insertBefore($('#choice-values')).hide().fadeIn('slow');
    

    typo: Waring.

  2. +++ b/poll.libraries.yml
    @@ -4,3 +4,7 @@ drupal.poll-links:
    +
    +poll:
    +  js:
    +      js/poll.js: {}
    

    lets give the file and library a better name. admin/poll_admin.js maybe. Or more specific to what we are doing.

  3. +++ b/src/Form/PollForm.php
    @@ -19,11 +19,20 @@ class PollForm extends ContentEntityForm {
     
    -    return parent::form($form, $form_state, $poll);
    +    foreach ($form['choice']['widget'] as $key => $choice) {
    +      if (is_int($key) && $form['choice']['widget'][$key]['choice']['#default_value'] != NULL) {
    +        $form['choice']['widget'][$key]['choice']['#attributes'] = ['class' => ['poll-existing-choice']];
    +      }
    +    }
    +
    +    $form['#attached'] = ['library' => ['poll/poll']];
    +
    

    We control the widget. Might be easier to just add it there?

    For the next time, look into Element::children(), then you don't need the is_int() check.

edurenye’s picture

Did those changes and started with tests, the problem is when I do $this->assertFalse($page->hasContent()), I want to check that there is no content, but it gets stuck there, not sure if there is another way to do it, I'm using it wrong or is a issue with mink or with core.

I upload it, maybe somebody else find a way to fix this test.

edurenye’s picture

Status: Needs review » Needs work

Locally the test was failing.

  • Berdir committed 2a797a1 on 8.x-1.x authored by edurenye
    Issue #2648836 by edurenye, Bambell: Alert the user that deleting a...
Berdir’s picture

Status: Needs work » Fixed

Works fine for me as well and fails as a test-only. Committed.

Berdir’s picture

Status: Fixed » Needs work

Actually, looks like it only works most of the time with PHP 7 (but not always).

Reverted.

The last submitted patch, 7: alert_the_user_that-2648836-7.patch, failed testing.

  • Berdir committed 5c95190 on 8.x-1.x
    Revert "Issue #2648836 by edurenye, Bambell: Alert the user that...

  • Berdir committed 1594e4a on 8.x-1.x authored by edurenye
    Issue #2648836 by edurenye, Bambell: Alert the user that deleting a...
Berdir’s picture

Status: Needs work » Fixed

Apparently this is failing somewhat randomly. Decided to commit the patch without test and opened #2736863: Add tests: Alert the user that deleting a choice will delete the votes on it to investigate and fix the test.

Status: Fixed » Closed (fixed)

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