Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

Core components content fragment handling fixes #391

Merged
merged 8 commits into from
Oct 22, 2019
Merged

Core components content fragment handling fixes #391

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f65d045
Fix handling of Textfield to enable configuration for the Content Fra…
Oct 10, 2019
1a594fb
Fix an error in the DefaultTextfield
Oct 10, 2019
bcc90e2
Merge branch 'master' into core-components-content-fragment-handling-…
Oct 10, 2019
2f0ea6a
Add NumberInput dialog field
Oct 14, 2019
a0a8c3d
Fix handling the content fragment warning dialog
Oct 14, 2019
0f9e02e
Fix most issues mentioned in CR
Oct 15, 2019
a09cf05
Fix the last of the issues mentioned in the CR
Oct 15, 2019
2d9b0c3
Fix the current year in copyright comments
Oct 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions ...ognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowserImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*-
* #%L
* Bobcat
* %%
* Copyright (C) 2016 Cognifide Ltd.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.cognifide.qa.bb.aem.core.component.dialog.dialogfields;

import static org.openqa.selenium.support.ui.ExpectedConditions.*;
kaczymuczy marked this conversation as resolved.
Show resolved Hide resolved

import java.util.List;
import java.util.NoSuchElementException;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.openqa.selenium.Keys;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;

import com.cognifide.qa.bb.qualifier.Global;
import com.cognifide.qa.bb.qualifier.PageObject;
import com.cognifide.qa.bb.wait.BobcatWait;
import com.cognifide.qa.bb.wait.TimingsBuilder;
import com.google.inject.Inject;

/**
* Default implementation of {@link PathBrowser}
*/
@PageObject(css = Locators.AUTOCOMPLETE_CSS)
public class ContentFragmentPathBrowserImpl implements ContentFragmentPathBrowser {
Copy link
Contributor

Choose a reason for hiding this comment

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

The DefaultXYZ naming is more aligned with the rest fields

Copy link
Author

Choose a reason for hiding this comment

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

Fixed


@FindBy(css = ".coral3-Textfield")
private WebElement input;

@FindBy(xpath = Locators.ALTERNATE_LABEL_XPATH)
private List<WebElement> label;

@FindBy(css = ".foundation-picker-buttonlist.coral3-Overlay.is-open")
private WebElement firstResult;

@Global
@FindBy(css = ".coral3-Dialog--warning.is-open .coral3-Button--primary")
private WebElement warningConfirmation;

@Inject
private BobcatWait bobcatWait;

@Override
public void setValue(Object value) {
input.clear();
input.sendKeys(String.valueOf(value));
bobcatWait.until(elementToBeClickable(firstResult));
input.sendKeys(Keys.ENTER);

// Branching logic in tests is not OK, it's true. But this part handles an unpredictable part of
// the application. Sometimes a warning dialog appears and it needs to be handled.
if (bobcatWait
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Try to extract the logic into a self-describing method instead of commenting.
  2. About the logic itself: are you trying to wait for the warning message -> confirm it -> verify the message disappeared?
    Few comments:
  • you are ignoring the results of the last isConditionMet - you burn some CPU time here, nothing more.
  • why the if clause in the first place? Does the message not appear every time? How about a following flow: wait for message to appear -> click it -> wait for it to disappear?

Copy link
Author

Choose a reason for hiding this comment

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

OK, will move to separate method.
The warning message doesn't always appear and I don't know yet what causes it to appear.
I know I'm ignoring the results of isConditionMet - it's a pretty sleep, nothing more, but it helps me avoid handling Selenium Exceptions

.tweak(new TimingsBuilder().explicitTimeout(1).build())
.ignoring(Stream.of(NoSuchElementException.class).collect(Collectors.toList()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a stream?
You can use: ignoring(NoSuchElementException.class)

Copy link
Author

Choose a reason for hiding this comment

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

I was testing this solution and thought that I might add more Exceptions later. But I didn't :D

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

.isConditionMet(elementToBeClickable(warningConfirmation))) {
warningConfirmation.click();
bobcatWait.tweak(new TimingsBuilder().explicitTimeout(1).build())
.isConditionMet(not(elementToBeClickable(warningConfirmation)));
}
}

@Override
public String getLabel() {
return label.isEmpty() ? "" : label.get(0).getText();
}
}
51 changes: 51 additions & 0 deletions ...n/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultNumberInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*-
* #%L
* Bobcat
* %%
* Copyright (C) 2016 Cognifide Ltd.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.cognifide.qa.bb.aem.core.component.dialog.dialogfields;

import java.util.List;

import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;

import com.cognifide.qa.bb.qualifier.PageObject;

/**
* Default implementation of {@link NumberInput}
*/
@PageObject(css = Locators.NUMBERINPUT_CSS)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the locator is not used anywhere else, put it here directly.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

public class DefaultNumberInput implements NumberInput {

@FindBy(css = "input")
private WebElement input;

@FindBy(xpath = Locators.ALTERNATE_LABEL_XPATH)
private List<WebElement> label;

@Override
public void setValue(Object value) {
input.clear();
input.sendKeys(String.valueOf(value));
}

@Override
public String getLabel() {
return label.isEmpty() ? "" : label.get(0).getText();
}
}
5 changes: 3 additions & 2 deletions ...n/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultPathBrowser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.util.List;

import org.openqa.selenium.Keys;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;

Expand Down Expand Up @@ -52,8 +53,8 @@ public class DefaultPathBrowser implements PathBrowser {
public void setValue(Object value) {
input.clear();
input.sendKeys(String.valueOf(value));

bobcatWait.until(elementToBeClickable(firstResult)).click();
bobcatWait.until(elementToBeClickable(firstResult));
input.sendKeys(Keys.ENTER);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change from the previous implementation?

Copy link
Author

Choose a reason for hiding this comment

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

The previous implementation didn't support external links, like to github.com - BB would click the first result suggestem by AEM, so a page within the AEM instance instead of github.com

}

@Override
Expand Down
13 changes: 10 additions & 3 deletions ...c/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultSelect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,29 @@
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;

import com.cognifide.qa.bb.qualifier.CurrentScope;
import com.cognifide.qa.bb.qualifier.PageObject;
import com.cognifide.qa.bb.wait.BobcatWait;
import com.google.inject.Inject;

/**
* Default implementation of {@link Select}
*/
@PageObject(xpath = "//coral-select/..")
@PageObject(css = Locators.SELECT_CSS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above - if this is the only place where the selector is used, leave it here.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

public class DefaultSelect implements Select {

private static final String SELECT_OPTIONS_CSS = ".coral3-SelectList-item";

@FindBy(css = ".coral3-Select")
@Inject
@CurrentScope
private WebElement selectField;

@FindBy(css = Locators.LABEL_CSS)
@FindBy(xpath = Locators.ALTERNATE_LABEL_XPATH)
private List<WebElement> label;

@Inject
private BobcatWait wait;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not used?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed


@Override
public void setValue(Object value) {
selectField.click();
Expand Down
9 changes: 6 additions & 3 deletions ...ain/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/DefaultTextfield.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,21 @@
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;

import com.cognifide.qa.bb.qualifier.CurrentScope;
import com.cognifide.qa.bb.qualifier.PageObject;
import com.google.inject.Inject;

/**
* Default implementation of {@link Textfield}
*/
@PageObject(css = Locators.FIELD_WRAPPER_CSS)
@PageObject(css = Locators.TEXTFIELD_CSS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment regarding locator

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

public class DefaultTextfield implements Textfield {

@FindBy(css = ".coral3-Textfield:not([type='hidden']")
@Inject
@CurrentScope
private WebElement input;

@FindBy(css = Locators.LABEL_CSS)
@FindBy(xpath = Locators.ALTERNATE_LABEL_XPATH)
private List<WebElement> label;

@Override
Expand Down
2 changes: 2 additions & 0 deletions ...m-65/src/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/Fields.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public class Fields {
public static final String RADIO_GROUP = "RADIO_GROUP";
public static final String RICHTEXT = "RICHTEXT";
public static final String TAGBROWSER = "TAGBROWSER";
public static final String NUMBER_INPUT = "NUMBER_INPUT";
public static final String CONTENT_FRAGMENT_PATHBROWSER = "CONTENT_FRAGMENT_PATHBROWSER";

private Fields() {
//empty
Expand Down
3 changes: 3 additions & 0 deletions ...65/src/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/Locators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ private Locators() {
public static final String MULTIFIELD_CSS = "coral-multifield";
public static final String AUTOCOMPLETE_CSS = "foundation-autocomplete";
public static final String ALTERNATE_LABEL_XPATH = "../label";
public static final String TEXTFIELD_CSS = ".coral3-Textfield:not([type='hidden'])";
public static final String SELECT_CSS = ".coral3-Select";
public static final String NUMBERINPUT_CSS = "coral-numberinput";
}
6 changes: 6 additions & 0 deletions bb-aem-65/src/main/java/com/cognifide/qa/bb/aem/core/modules/fields/DialogFieldsModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@
import org.slf4j.LoggerFactory;

import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.Checkbox;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.ContentFragmentPathBrowser;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.ContentFragmentPathBrowserImpl;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultCheckbox;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultImage;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultMultifield;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultMultifieldItem;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultNumberInput;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultPathBrowser;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultRadioGroup;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultRichText;
Expand All @@ -35,6 +38,7 @@
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.Image;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.Multifield;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.MultifieldItem;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.NumberInput;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.PathBrowser;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.RadioGroup;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.RichText;
Expand Down Expand Up @@ -62,6 +66,8 @@ protected void configure() {
bind(RichText.class).to(DefaultRichText.class);
bind(Select.class).to(DefaultSelect.class);
bind(Textfield.class).to(DefaultTextfield.class);
bind(NumberInput.class).to(DefaultNumberInput.class);
bind(ContentFragmentPathBrowser.class).to(ContentFragmentPathBrowserImpl.class);

install(new FieldsRegistryModule());
}
Expand Down
6 changes: 6 additions & 0 deletions ...em-65/src/main/java/com/cognifide/qa/bb/aem/core/modules/fields/FieldsRegistryModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.ContentFragmentPathBrowserImpl;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultCheckbox;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultImage;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultMultifield;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultMultifieldItem;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultNumberInput;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultPathBrowser;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultRadioGroup;
import com.cognifide.qa.bb.aem.core.component.dialog.dialogfields.DefaultRichText;
Expand Down Expand Up @@ -62,12 +64,16 @@ protected void configure() {
registerField(fieldsBinder, Fields.MULTIFIELD_ITEM, DefaultMultifieldItem.class);
registerField(fieldsBinder, Fields.RADIO_GROUP, DefaultRadioGroup.class);
registerField(fieldsBinder, Fields.TAGBROWSER, DefaultTagBrowser.class);
registerField(fieldsBinder, Fields.NUMBER_INPUT, DefaultNumberInput.class);

registerField(fieldsBinder, Fields.RICHTEXT, DefaultRichText.class);
registerField(fieldsBinder, Options.RTE_OPTIONS, RteOption.class);
registerField(fieldsBinder, Options.RTE_OPTIONS_HYPERLINK, Hyperlink.class);
registerField(fieldsBinder, Options.RTE_OPTIONS_LISTS, Lists.class);
registerField(fieldsBinder, Options.RTE_OPTIONS_PARAGRAPH_FORMATS, ParagraphFormats.class);

registerField(fieldsBinder, Fields.CONTENT_FRAGMENT_PATHBROWSER,
ContentFragmentPathBrowserImpl.class);
}

private void registerField(MapBinder<String, DialogField> binder, String name,
Expand Down
29 changes: 29 additions & 0 deletions ...om/cognifide/qa/bb/aem/core/component/dialog/dialogfields/ContentFragmentPathBrowser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*-
* #%L
* Bobcat
* %%
* Copyright (C) 2019 Cognifide Ltd.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.cognifide.qa.bb.aem.core.component.dialog.dialogfields;

import com.cognifide.qa.bb.qualifier.PageObjectInterface;

/**
* This class represents path browser dialog field.
*/
@PageObjectInterface
public interface ContentFragmentPathBrowser extends DialogField {
}
29 changes: 29 additions & 0 deletions ...src/main/java/com/cognifide/qa/bb/aem/core/component/dialog/dialogfields/NumberInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*-
* #%L
* Bobcat
* %%
* Copyright (C) 2019 Cognifide Ltd.
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package com.cognifide.qa.bb.aem.core.component.dialog.dialogfields;

import com.cognifide.qa.bb.qualifier.PageObjectInterface;

/**
* This class represents single line number input dialog field.
*/
@PageObjectInterface
public interface NumberInput extends DialogField {
}