Problem/Motivation

The toolbar integration uses 'N changes' even if there is only 1 change.

Steps to reproduce

Proposed resolution

Use format plural functionality to show '1 change' or 'N changes'

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 interdiff_8-11.txt688 bytesmunish.kumar
#11 3172128-11.patch3.3 KBmunish.kumar
#8 interdiff_5-8.txt2.29 KBmunish.kumar
#8 3172128-8.patch3.03 KBmunish.kumar
#5 3172128-5.patch2.19 KBPooja Ganjage
#2 3172128-2.patch583 bytesPooja Ganjage
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan created an issue. See original summary.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3172128-2.patch583 bytes

Hi,

I am creating patch for this issue.

Kindly review the patch once.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3172128-2.patch, failed testing. View results

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3172128-5.patch2.19 KB
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Needs work 
+++ b/build_hooks.module
@@ -104,7 +104,7 @@ function build_hooks_toolbar() {
+            '#title' => t('@envName (@num change)', [

@@ -122,7 +122,7 @@ function build_hooks_toolbar() {
+          '#title' => t('@envName (@num change)', [

@@ -135,7 +135,7 @@ function build_hooks_toolbar() {
+          '#title' => $totalChanges > 0 ? t('Deployments (@num total change)', ['@num' => $totalChanges]) : t('Deployments'),

This needs to use format_plural instead of t - because now when there 3 changes, it will say '3 change' instead of '3 changes;

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3172128-8.patch3.03 KB
interdiff_5-8.txt2.29 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Thanks @munish.kumar!

Do you think we should expand that test to assert that it shows '2 changes' when another item is added - would be something like this added to the end of the existing assertToolbarIntegration method:

$label = $this->randomMachineName();
    $entity = EntityTest::create([
      'name' => $label,
    ]);
    $entity->save();
$this->drupalGet(Url::fromRoute('<front>'));
    $this->assertSession()->linkExists(sprintf('%s (2 changes)', $environment->label()));

Thoughts?

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented

Hi @larowlan, Agree with your thoughts, That we should need the test cases for the multiple changes. We need to expand the existing test case so that we can cover both the scenario. Will update the patch shortly.

Thanks for your valuable feedback.

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
FileSize
3172128-11.patch3.3 KB
interdiff_8-11.txt688 bytes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Thanks @munish.kumar - fixed!

Really appreciate all the patches here.

  • larowlan committed 1179b48 on 8.x-2.x authored by munish.kumar 
    Issue #3172128 by munish.kumar, Pooja Ganjage, larowlan: Toolbar button...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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