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

Custom Policy Deletion #723

Merged
merged 5 commits into from
Mar 27, 2019
Merged

Conversation

didierofrivia
Copy link
Member

@didierofrivia didierofrivia commented Mar 26, 2019

Removing Policy not in use

deleting-policy

Removing Policy in use is not possible

deleting-policy-avoided

What this PR does / why we need it:

Makes it possible to delete a policy from the UI

Which issue(s) this PR fixes
fixes https://issues.jboss.org/browse/THREESCALE-2100

Verification steps

  • Go to Custom Policies
  • Edit or Create a new one
  • A delete button should be visible when editing
  • Click and it should remove the policy if not in use.
  • Errors should be shown if it's used by the Policy Chain

Special notes for your reviewer:

  • Styling will be done in a separated PR
  • A test is skipped until a PR by Enzyme is merged.

@didierofrivia didierofrivia self-assigned this Mar 26, 2019
@didierofrivia didierofrivia force-pushed the feature/custom-policy-delete-button branch from 54bdf14 to 40859b6 Compare March 26, 2019 10:24
@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@0b99c50). Click here to learn what that means.
The diff coverage is 94.73%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #723   +/-   ##
=========================================
  Coverage          ?   92.86%           
=========================================
  Files             ?     2388           
  Lines             ?    77734           
  Branches          ?        0           
=========================================
  Hits              ?    72185           
  Misses            ?     5549           
  Partials          ?        0
Impacted Files Coverage Δ
...tion/provider/registry/policies_controller_test.rb 100% <100%> (ø)
...ers/provider/admin/registry/policies_controller.rb 93.54% <85.71%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b99c50...872bc33. Read the comment docs.

@didierofrivia didierofrivia changed the title [WIP] Custom Policy Deletion Custom Policy Deletion Mar 27, 2019
@didierofrivia didierofrivia force-pushed the feature/custom-policy-delete-button branch from 44eba3c to d09e1f1 Compare March 27, 2019 08:24
@didierofrivia didierofrivia changed the title Custom Policy Deletion [WIP] Custom Policy Deletion Mar 27, 2019
@@ -66,3 +66,5 @@ export type ElementEventTemplate<E> = {
} & Event & SyntheticEvent<>

export type InputEvent = ElementEventTemplate<HTMLInputElement>

export type Window = any
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go in libdefs.js?

@@ -44,9 +44,18 @@ function CSRFToken ({win = window}: {win?: any}) {
)
}

function CustomPolicyForm ({policy, onChange}: {policy: Policy, onChange: OnChange}) {
function CustomPolicyForm ({policy, onChange, win = window}: {policy: Policy, onChange: OnChange, win?: Window}) {
const isNewPolicy = !(policy.id && policy.id !== 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

To fix codeclimate complexity error, maybe

Suggested change
const isNewPolicy = !(policy.id && policy.id !== 0)
const isNewPolicy = !policy.id

@didierofrivia didierofrivia force-pushed the feature/custom-policy-delete-button branch 2 times, most recently from 8fe7a31 to 6a78e20 Compare March 27, 2019 11:41
josemigallas
josemigallas previously approved these changes Mar 27, 2019
@thomasmaas thomasmaas changed the title [WIP] Custom Policy Deletion Custom Policy Deletion Mar 27, 2019
thomasmaas
thomasmaas previously approved these changes Mar 27, 2019
@@ -43,11 +43,21 @@ def create
end
end

def edit; end
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need this I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess code climate will complain without it :/

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 are right and Code Climate should not complain. I will remove

Copy link
Member Author

Choose a reason for hiding this comment

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

Codeclimate complained about it, that was why hehe

damianpm
damianpm previously approved these changes Mar 27, 2019
thomasmaas
thomasmaas previously approved these changes Mar 27, 2019
@thomasmaas thomasmaas merged commit 08bd3d7 into master Mar 27, 2019
@guicassolato guicassolato deleted the feature/custom-policy-delete-button branch March 27, 2019 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants