Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

ISAICP-6577: Topic select to use SlimSelect. #2592

Open
wants to merge 24 commits into
base: EPIC-6572
Choose a base branch
from
Open

Conversation

saidatom
Copy link
Contributor

No description provided.

Base automatically changed from ISAICP-6584 to ISAICP-6415 September 24, 2021 05:45
@saidatom saidatom changed the base branch from ISAICP-6415 to EPIC-6572 September 28, 2021 13:56
@saidatom saidatom changed the base branch from EPIC-6572 to ISAICP-6415 September 28, 2021 14:00
@saidatom saidatom changed the base branch from ISAICP-6415 to EPIC-6572 September 30, 2021 12:16
@saidatom saidatom changed the base branch from EPIC-6572 to ISAICP-6415-backup October 1, 2021 12:32
@saidatom saidatom changed the base branch from ISAICP-6415-backup to EPIC-6572 October 1, 2021 12:46
@saidatom saidatom changed the base branch from EPIC-6572 to ISAICP-6688 October 1, 2021 12:51
Base automatically changed from ISAICP-6688 to EPIC-6572 October 13, 2021 16:16
Copy link
Contributor

@claudiu-cristea claudiu-cristea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before doing a detailed QA, we need to fix:

  • Looking to mockups, at https://citnet.tech.ec.europa.eu/CITnet/jira/browse/ISAICP-6572 (multiselect - Desktop.pdf), I see there is a multiselect Slim Select. But here we have a single select. Should be fixed.
  • Tests are failing. I tried locally and failed for me too, but in a different line. We should find out why and fix. The fact that it passes locally and failing in CPHP cannot be explained generically with "infrastructure issue". It's new test coverage and should pass.

web/modules/custom/joinup_search/joinup_search.module Outdated Show resolved Hide resolved
Copy link
Contributor

@claudiu-cristea claudiu-cristea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the inline remarks, we need to test multi-select.

tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
tests/features/joinup_search/search.feature Show resolved Hide resolved
tests/src/Context/JoinupSearchContext.php Outdated Show resolved Hide resolved
*
* @When I select :option option(s) from the :label Slim Select
*/
public function iSelectAnOptionFromSlimSelect(string $option, string $label): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a/iSelectAnOptionFromSlimSelect/iSelectOptionsFromSlimSelect

Comment on lines 346 to 352
$facet_id = self::getFacetIdFromAlias($label);
$session = $this->getSession();
$xpath = '//div[contains(concat(" ", normalize-space(@class), " "), " block-facets-form ")]//select[@data-drupal-selector="' . $facet_id . '"]';
$slim_select = $session->getPage()->find('xpath', $xpath);
if (!$slim_select) {
throw new \Exception("The Slim Select '$label' was not found in the page.");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of abstracting the Slim Select search/find into a protected method was not implemented. Also we have here hadcoded references to Facets Form. We need a generic step definition that work not only for Facets Form but for any Slim Select on the site. Should be only searchable by its label: get the native select > (wait to be rendered) > get the Slim Select.

Comment on lines 475 to 482
$facet_id = self::getFacetIdFromAlias($select);
$region = $this->getSession()->getPage();
$slim_select = $region->find('xpath', "//*[@data-drupal-selector='{$facet_id}']");

if (!$slim_select) {
throw new \Exception(sprintf('The select "%s" was not found in the page %s', $select, $this->getSession()->getCurrentUrl()));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. Need abstraction.

Comment on lines 578 to 581
$facet_id = self::getFacetIdFromAlias($select);
$region = $this->getSession()->getPage();
$xpath = "//*[@data-drupal-selector='{$facet_id}']";
$page = $this->getSession()->getPage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. Abstraction

Comment on lines 501 to 676
public function iClickActionsInFacetsForm(string $name) {
public function iClickSearchInFacetsForm(string $name) {
$region = $this->getSession()->getPage();
$element = $region->find('xpath', "//input[@value='{$name}']");
$element->click();
$xpath = '//div[contains(concat(" ", normalize-space(@class), " "), " block-facets-form ")]//div[@data-drupal-selector="edit-actions"]';
$actions = $region->find('xpath', $xpath);

$element = $actions->find('css', "input[value|='{$name}']");
$element->submit();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this. Use When I press "Search".

$slim_select = $region->find('xpath', "//*[@data-drupal-selector='{$facet_id}']");

if (!$slim_select) {
throw new \Exception(sprintf('The select "%s" was not found in the page %s', $select, $this->getSession()->getCurrentUrl()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ElementNotFoundException everywhere it applies.

Comment on lines 355 to 358
$xpath = '//div[contains(concat(" ", normalize-space(@class), " "), " block-facets-form ")]//div[contains(concat(" ", normalize-space(@class), " "), "' . $slim_select->getAttribute('data-ssid') . '")]';
$select = $session->getPage()->waitFor(5, function () use ($session, $xpath) {
return $session->getPage()->find('xpath', $xpath);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the new findSlimSelect() methods so it applies everywhere we need to get the Slim Select.

throw new \Exception(sprintf('The select "%s" was not found in the page %s', $select, $this->getSession()->getCurrentUrl()));
}

if ($this->browserSupportsJavaScript()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Slim Select step definitions should only work with Selenium tests. Use \Drupal\joinup\Traits\MaterialDesignTrait::selectMaterialDesignRadioButton() as an example on how to enforce this.

For the test that runs w/o JS (tests/features/homepage.feature) use the @Then the option with text :option from select :select is selected step def.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants