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

Do not allow user to attempt to delete a running job #9758

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

nixocio
Copy link
Contributor

@nixocio nixocio commented Mar 30, 2021

Do not allow user to attempt to delete a running job

See: #9187

image

@@ -238,6 +238,13 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) {
onDelete={handleJobDelete}
itemsToDelete={selected}
pluralizedItemName={i18n._(t`Jobs`)}
cannotDelete={item =>
item.status === 'running' ||
Copy link
Contributor

Choose a reason for hiding this comment

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

There's probably more than one status we'll need to check for. Let's use the isJobRunning util: https://github.com/ansible/awx/blob/devel/awx/ui_next/src/util/jobs.js#L10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will also try to use the plural for the error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakemcdermott, updated.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@@ -238,6 +240,19 @@ function JobList({ i18n, defaultParams, showTypeColumn = false }) {
onDelete={handleJobDelete}
itemsToDelete={selected}
pluralizedItemName={i18n._(t`Jobs`)}
cannotDelete={item =>
Copy link
Member

Choose a reason for hiding this comment

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

Since pluralization can be complicated its best to use this syntax.

Once we upgrade Lingui we will adopt <Plural/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexSCorey, I saw this syntax. And I attempted to use it. But the string reported during unit-tests was not sane. I will take a better look if I missed something.

@tiagodread tiagodread self-assigned this Apr 5, 2021
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Do not allow user to attempt to delete a running job

See: ansible#9187
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@tiagodread tiagodread left a comment

Choose a reason for hiding this comment

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

Worked as expected 👍🏽

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 229a3d6 into ansible:devel Apr 6, 2021
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.

4 participants