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.

Sign up for GitHub

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

Jump to bottom

[CLOUDTRUST-2109] PR2 authorization management #158

Merged
merged 4 commits into from
Feb 19, 2020
Merged

[CLOUDTRUST-2109] PR2 authorization management #158

merged 4 commits into from
Feb 19, 2020

Conversation

harture
Copy link
Contributor

@harture harture commented Jan 20, 2020 •
edited by fperot74

@coveralls
Copy link

coveralls commented Jan 20, 2020 •
edited

Pull Request Test Coverage Report for Build 1855

  • 29 of 38 (76.32%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 91.454%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/keycloakb/configdbinstrumenting.go 2 4 50.0%
internal/keycloakb/configdbmodule.go 8 15 53.33%
Totals Coverage Status
Change from base Build 1844: -0.01%
Covered Lines: 4773
Relevant Lines: 5219
💛 - Coveralls

@harture harture force-pushed the CLOUDTRUST-2109_2_authorizationManagement branch 3 times, most recently from 83515f2 to aaa70aa Compare January 21, 2020 12:47
bsoniam
bsoniam approved these changes Jan 22, 2020
View reviewed changes
Copy link
Contributor

@bsoniam bsoniam left a comment

Choose a reason for hiding this comment

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

Your build seems to fail because of the failed tests

@harture
Copy link
Contributor Author

harture commented Jan 23, 2020

Check all swagger for /actions

@harture
Copy link
Contributor Author

harture commented Jan 23, 2020

Remove Name in messages.go
In api.go -> l:441 internal.GroupName instead of internal.GroupID

@harture
Copy link
Contributor Author

harture commented Jan 23, 2020

Move ActionRepresentation into common.
TODO in all repos (i.e. also in papercard)

@harture harture mentioned this pull request Jan 29, 2020
Merged
@harture harture force-pushed the CLOUDTRUST-2109_2_authorizationManagement branch 2 times, most recently from 103664d to b4dd361 Compare February 10, 2020 21:19
@harture harture requested a review from bsoniam February 11, 2020 14:50
@harture
Copy link
Contributor Author

harture commented Feb 11, 2020

@bsoniam @fperot74 Could you review it again ? I did rebase and we are now ready for merge.

bsoniam
bsoniam approved these changes Feb 11, 2020
View reviewed changes
Copy link
Contributor

@bsoniam bsoniam left a comment

Choose a reason for hiding this comment

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

All seems ok for me.

pkg/events/authorization_test.go Outdated
}
}
}`
const ()
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this?

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 think we can safely remove it ;)

api/event/fb/AdminEvent.go Outdated
return rcv._tab.MutateInt8Slot(14, n)
}

func (rcv *AdminEvent) OperationType() OperationType {
func (rcv *AdminEvent) OperationType() int8 {
Copy link
Contributor

Choose a reason for hiding this comment

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

this change is related to the PR that added information on the admin event ?

fperot74
fperot74 approved these changes Feb 12, 2020
View reviewed changes
Gopkg.toml Outdated
@@ -27,11 +27,11 @@

[[constraint]]
name = "github.com/cloudtrust/common-service"
version = "v1.2.4"
branch = "CLOUDTRUST-2109_2_authorizationManagement"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to update before merging

Gopkg.toml Outdated

[[constraint]]
name = "github.com/cloudtrust/keycloak-client"
version = "v1.2.7"
branch = "CLOUDTRUST-2109_2_branch"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to update before merging

@harture harture force-pushed the CLOUDTRUST-2109_2_authorizationManagement branch from aaa6539 to 146e4be Compare February 19, 2020 22:58
@harture harture force-pushed the CLOUDTRUST-2109_2_authorizationManagement branch from 146e4be to 0c73a4b Compare February 19, 2020 23:15
@harture harture merged commit 0e44c68 into master Feb 19, 2020
@fperot74 fperot74 deleted the CLOUDTRUST-2109_2_authorizationManagement branch January 6, 2021 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsoniam bsoniam bsoniam approved these changes

@fperot74 fperot74 fperot74 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@harture @coveralls @bsoniam @fperot74