-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OPENEUROPA-1328: Media entity browser handling for AV Portal media. #12
Conversation
08c6114
to
0689dcf
Compare
0689dcf
to
3ef013d
Compare
e753487
to
96f0489
Compare
96f0489
to
b8b17cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code works and the behat tests could be improved, nothing blocker.
And I visit "node/add/oe_media_demo" | ||
And I fill in "Title" with "Media demo" | ||
And I click the fieldset "Media browser field" | ||
When I press the "Select entities" button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
should be And
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. The pressing is the trigger that should then then call for the "then". So it's correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if it's the case, please insert a then
before.
According the documentation http://docs.behat.org/en/v2.5/quick_intro.html
Each part of the scenario - the context, the event, and the outcome - can be extended by adding the And or But keyword:
Scenario: Some description of the scenario
Given [some context]
And [more context]
When [some event]
And [second event occurs]
Then [outcome]
And [another outcome]
But [another outcome]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too late, it's merged already.
|
||
@javascript @av_portal | ||
Scenario: The node adding form should contain entity browser widget with possibility to add new and reuse existing AV Portal video. | ||
Given I am logged in as a user with the "create oe_media_demo content,create av_portal_video media,access media_entity_browser entity browser pages" permission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Given
should be in the Background
of this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@voidtek Why? There is only one scenario
# Cleanup of the media entity. | ||
And I remove the media "Midday press briefing from 25/10/2018" | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove these empty lines.
And I press the "Save" button | ||
Then I should see the AV Portal video "Midday press briefing from 25/10/2018" | ||
# Cleanup of the media entity. | ||
And I remove the media "Midday press briefing from 25/10/2018" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Clean up should be done on an @AfterScenario
step.
This changes can be done on a follow-up ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Will create the follow up for this.
The scope of this ticket is to expose the AV Portal media entities to the generic entity browser in the demo module.
Todo: