Skip to content
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

Display WF always nodes in conjunction with success and failure #2373

Merged

Conversation

marshmalien
Copy link
Member

SUMMARY

Link: #2255 #2288 #2289

  • Remove edge conflict logic and styles
  • Remove sibling connection types logic
  • Show all edge type options (always, success, fail) when node is not a root node
  • Audit dropdown label strings

2018-10-04 23-20-19 2018-10-04 23_23_15

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • UI

rooftopcellist and others added 3 commits October 9, 2018 11:30
* Edge type of root node is always "always"
* If node is not a root node, show all options: always, success, fail
* Remove edge conflict logic
Copy link
Member

@chrismeyersfsu chrismeyersfsu left a comment

Choose a reason for hiding this comment

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

Make sure test_node_triggers_should_be_mutually_exclusive in tower-qa is updated appropriately.

@@ -3438,10 +3438,6 @@ def get_queryset(self):
return getattr(parent, self.relationship).all()

def is_valid_relation(self, parent, sub, created=False):
mutex_list = ('success_nodes', 'failure_nodes') if self.relationship == 'always_nodes' else ('always_nodes',)
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

Can confirm, this test is currently failing on the always_nodes_ui branch:

py.test -c config/docker.cfg --base-url='http://localhost:8013' -k 'test_node_triggers_should_be_mutually_exclusive' --disable-pytest-warnings

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

// This finds the credentials that were selected in the prompt but don't occur
// in the template defaults
let credentialsToPost = params.node.promptData.prompts.credentials.value.filter(function (credFromPrompt) {
let defaultCreds = params.node.promptData.launchConf.defaults.credentials ? params.node.promptData.launchConf.defaults.credentials : [];
Copy link
Contributor

Choose a reason for hiding this comment

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

can also do _.get(params, 'node.promptData.launchConf.defaults.credentials', []) for this kind of thing.

.then(function() {
$scope.closeDialog();
}).catch(({data, status}) => {
$q.all(associatePromises.concat(credentialPromises))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can avoid having to duplicate the catch block in the inner promise by adding a return statement here.

}, function (error) {
ProcessErrors($scope, error.data, error.status, null, {
hdr: 'Error!',
msg: 'Failed to add workflow node. ' +
Copy link
Contributor

Choose a reason for hiding this comment

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

strings / i18n?

@ryanpetrello
Copy link
Contributor

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link

@unlikelyzero unlikelyzero left a comment

Choose a reason for hiding this comment

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

Approving, but let's talk about this

client
.login()
.waitForAngular()
.resizeWindow(1200, 1000)

Choose a reason for hiding this comment

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

let's avoid doing this midtest. We can launch chrome with a certain window size if it's necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed this happens across a ton of the UI tests ~15 times. I've made #2426 to track the issue to be handled en masse.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 73f54b2 into ansible:devel Oct 12, 2018
@marshmalien marshmalien deleted the always_nodes_ui branch October 11, 2019 17:02
sganesh999 pushed a commit to sganesh999/awx that referenced this pull request Mar 4, 2020
Display WF always nodes in conjunction with success and failure

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants