Comments

naveenvalecha created an issue. See original summary.

yanniboi’s picture

Status: Active » Needs review
StatusFileSize
new3.93 KB

So it turns out that there is missing schema for instagram block settings (ie. count, width, height, etc).

This is a test that should fail because of missing schema.

yanniboi’s picture

StatusFileSize
new4.75 KB

I've added the schema for this patch so it should pass.

yanniboi’s picture

Normally I would want a separate issue for the schema fix, but since we are playing catchup with test coverage, I will be leaving it in this issue.

The last submitted patch, 2: add_test_coverage_for-2795183-2.patch, failed testing.

naveenvalecha’s picture

Status: Needs review » Needs work
  1. +++ b/config/schema/instagram_block.schema.yml
    @@ -8,3 +8,23 @@ instagram_block.settings:
    +block.settings.instagram_block_block:
    +  type: block_settings
    +  label: 'Instagram block'
    +  mapping:
    +    count:
    +      type: integer
    +      label: 'Number of images to display'
    +    width:
    +      type: integer
    +      label: 'Image width in pixels'
    +    height:
    +      type: integer
    +      label: 'Image height in pixels'
    +    img_resolution:
    +      type: string
    +      label: 'Image resolution'
    +    cache_time_minutes:
    +      type: integer
    +      label: 'Cache time in minutes'
    

    We don't need this schema

  2. +++ b/src/Tests/InstagramBlockPlaceBlockTest.php
    @@ -0,0 +1,117 @@
    +    // Check that there are no blocks in the header region.
    +    $element = $this->xpath('//tr[contains(@class, :class1) and contains(@class, :class2)]', [
    +      ':class1' => 'region-header-message',
    +      ':class2' => 'region-empty',
    +    ]);
    +    $this->assertTrue(!empty($element));
    +
    +    // Place an instagram block in the header region.
    +    $values = [
    +      'plugin_id' => 'instagram_block_block',
    +      'settings' => array('region' => 'header', 'id' => 'instagramblock'),
    +    ];
    +    $this->drupalPlaceBlock($values['plugin_id'], $values['settings']);
    +
    +    // Check that the block was placed successfully.
    +    $this->drupalGet('admin/structure/block');
    +    $element = $this->xpath('//tr[contains(@class, :class1) and contains(@class, :class2)]', [
    +      ':class1' => 'region-header-message',
    +      ':class2' => 'region-populated',
    +    ]);
    +    $this->assertTrue(!empty($element));
    +
    +    // Test context mapping with valid data.
    +    $this->drupalGet('admin/structure/block/manage/instagramblock');
    +    $edit = [
    +      'settings[count]' => '4',
    +      'settings[width]' => '150',
    +      'settings[height]' => '150',
    +      'settings[img_resolution]' => 'thumbnail',
    +      'settings[cache_time_minutes]' => '1440',
    +    ];
    +    $this->drupalPostForm(NULL, $edit, 'Save block');
    +    $this->assertText('The block configuration has been saved.', 'Block save without errors.');
    +
    +    // Test count with invalid data.
    +    $edit = [
    +      'settings[count]' => $this->randomString(4),
    +    ];
    +    $this->drupalPostForm('admin/structure/block/manage/instagramblock', $edit, 'Save block');
    +    $this->assertText('Number of images to display. must be a number.', 'Block failed to save.');
    +
    +    // Test width with invalid data.
    +    $edit = [
    +      'settings[width]' => $this->randomString(4),
    +    ];
    +    $this->drupalPostForm('admin/structure/block/manage/instagramblock', $edit, 'Save block');
    +    $this->assertText('Image width in pixels. must be a number.', 'Block failed to save.');
    +
    +    // Test height with invalid data.
    +    $edit = [
    +      'settings[height]' => $this->randomString(4),
    +    ];
    +    $this->drupalPostForm('admin/structure/block/manage/instagramblock', $edit, 'Save block');
    +    $this->assertText('Image height in pixels. must be a number.', 'Block failed to save.');
    +
    +    // Test cache_time_minutes with invalid data.
    +    $edit = [
    +      'settings[cache_time_minutes]' => $this->randomString(4),
    +    ];
    +    $this->drupalPostForm('admin/structure/block/manage/instagramblock', $edit, 'Save block');
    +    $this->assertText('Cache time in minutes must be a number.', 'Block failed to save.');
    

    We need to do this test for all core themes.
    // Test availability of block in the admin 'Place blocks' list.
    \Drupal::service('theme_handler')->install(['bartik', 'seven', 'stark']);
    $theme_settings = $this->config('system.theme');
    foreach (['bartik', 'seven', 'stark'] as $theme) {
    ....
    }

    See reference how its done in sharethis module http://cgit.drupalcode.org/sharethis/tree/src/Tests/SharethisBlockTest.p...

yanniboi’s picture

We don't need this schema

Trust me, I have spent a lot of time on core schema bugs. We need this. If nothing else the fact that it fixes the test fail proves that.

yanniboi’s picture

Status: Needs work » Needs review
StatusFileSize
new5.18 KB
new853 bytes

Added test for checking 'instagram block' is a option in the 'place block' selection listings for core themes.

naveenvalecha’s picture

Trust me, I have spent a lot of time on core schema bugs. We need this. If nothing else the fact that it fixes the test fail proves that.

I'll cross check this later tonight
Edit : I have cross checked it its needed :) I was about to sleep that time, mind sucks sometime

yanniboi’s picture

Issue summary: View changes
StatusFileSize
new59.11 KB
new54.37 KB

No problem, here is some more reference:

This is how the aggregator module adds block settings schema in aggregator.links.menu.yml:

<?php
block.settings.aggregator_feed_block:
  type: block_settings
  label: 'Aggregator feed block'
  mapping:
    block_count:
      type: integer
      label: 'Block count'
    feed:
      type: string
      label: 'Feed'
?>

AggregatorFeedBlock.php

<?php
    $form['feed'] = array(
      '#type' => 'select',
      '#title' => $this->t('Select the feed that should be displayed'),
      '#default_value' => $this->configuration['feed'],
      '#options' => $options,
    );
    $range = range(2, 20);
    $form['block_count'] = array(
      '#type' => 'select',
      '#title' => $this->t('Number of news items in block'),
      '#default_value' => $this->configuration['block_count'],
      '#options' => array_combine($range, $range),
    );
?>

And also some screenshots of the output from config_inspector module with and without patch:

yanniboi’s picture

StatusFileSize
new4.36 KB

Status: Needs review » Needs work

The last submitted patch, 11: add_test_coverage_for-2795183-11.patch, failed testing.

yanniboi’s picture

Status: Needs work » Needs review
StatusFileSize
new5.15 KB
new2.79 KB
naveenvalecha’s picture

Status: Needs review » Needs work
  1. +++ b/src/Tests/InstagramBlockPlaceBlockTest.php
    @@ -0,0 +1,136 @@
    +   * A user with permission to administer instagram block.
    

    update the description here as the user has more administrative permisssions

  2. +++ b/src/Tests/InstagramBlockPlaceBlockTest.php
    @@ -0,0 +1,136 @@
    +    }
    

    We also need to place the blocks for these theme and check on the homepage/other node page that whether the block is showing/not

yanniboi’s picture

I don't think we can test actual rendering...

I am (obviously) not using actual instagram account user id and access token, so there is no actual response from instagram and it does not render the block in the test.

naveenvalecha’s picture

Status: Needs work » Reviewed & tested by the community

Let;s add a separate issue to discuss how can we test the rendering. Let's get this in

  • yanniboi committed 89a4efc on 8.x-2.x
    Issue #2795183 by yanniboi, naveenvalecha: Add test coverage for...
yanniboi’s picture

Status: Reviewed & tested by the community » Fixed
yanniboi’s picture

Status: Fixed » Closed (fixed)

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