-
Notifications
You must be signed in to change notification settings - Fork 8
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
CSS-9587 unique offer urls #1273
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
-- 1_11.sql is a migration that enforces uniqueness on URLs in application offers. | ||
ALTER TABLE application_offers ADD UNIQUE (url); | ||
|
||
UPDATE versions SET major=1, minor=11 WHERE component='jimmdb'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ package jimm | |
import ( | ||
"context" | ||
"database/sql" | ||
"fmt" | ||
"sort" | ||
"strings" | ||
|
||
|
@@ -68,6 +69,19 @@ func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer AddApplicati | |
ApplicationName: offer.OfferName, | ||
} | ||
|
||
// Verify offer URL doesn't already exist. | ||
var offerCheck dbmodel.ApplicationOffer | ||
offerCheck.URL = offerURL.String() | ||
err = j.Database.GetApplicationOffer(ctx, &offerCheck) | ||
if err == nil { | ||
Comment on lines
+75
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Since |
||
return errors.E(fmt.Sprintf("offer %s already exists, use a different name", offerURL.String()), errors.CodeAlreadyExists) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use a different offer name would be better language There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add this in a follow-up |
||
} else { | ||
if errors.ErrorCode(err) != errors.CodeNotFound { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a leftover after removing something? -> else if |
||
// Anything besides Not Found is a problem. | ||
return errors.E(op, err) | ||
} | ||
} | ||
|
||
api, err := j.dial(ctx, &model.Controller, names.ModelTag{}) | ||
if err != nil { | ||
return errors.E(op, err) | ||
|
@@ -108,10 +122,6 @@ func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer AddApplicati | |
|
||
var doc dbmodel.ApplicationOffer | ||
doc.FromJujuApplicationOfferAdminDetailsV5(offerDetails) | ||
if err != nil { | ||
zapctx.Error(ctx, "failed to convert application offer details", zaputil.Error(err)) | ||
return errors.E(op, err) | ||
} | ||
kian99 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
doc.ModelID = model.ID | ||
err = j.Database.Transaction(func(db *db.Database) error { | ||
if err := db.AddApplicationOffer(ctx, &doc); err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,7 @@ func (s *applicationOffersSuite) TestOffer(c *gc.C) { | |
c.Assert(results, gc.HasLen, 1) | ||
c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) | ||
|
||
results, err = client.Offer(s.Model.UUID.String, "no-such-app", []string{s.endpoint.Name}, "[email protected]", "test-offer", "test offer description") | ||
results, err = client.Offer(s.Model.UUID.String, "no-such-app", []string{s.endpoint.Name}, "[email protected]", "test-offer-foo", "test offer description") | ||
c.Assert(err, gc.Equals, nil) | ||
c.Assert(results, gc.HasLen, 1) | ||
c.Assert(results[0].Error, gc.Not(gc.IsNil)) | ||
|
@@ -83,6 +83,29 @@ func (s *applicationOffersSuite) TestOffer(c *gc.C) { | |
c.Assert(results[0].Error.Code, gc.Equals, "unauthorized access") | ||
} | ||
|
||
func (s *applicationOffersSuite) TestCreateMultipleOffersForSameApp(c *gc.C) { | ||
conn := s.open(c, nil, "[email protected]") | ||
defer conn.Close() | ||
client := applicationoffers.NewClient(conn) | ||
|
||
results, err := client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "[email protected]", "test-offer", "test offer description") | ||
c.Assert(err, gc.Equals, nil) | ||
c.Assert(results, gc.HasLen, 1) | ||
c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) | ||
|
||
// Creating an offer with the same name as above. | ||
results, err = client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "[email protected]", "test-offer", "test offer description") | ||
c.Assert(err, gc.Equals, nil) | ||
c.Assert(results, gc.HasLen, 1) | ||
c.Assert(results[0].Error, gc.ErrorMatches, `offer [email protected]/model-1.test-offer already exists, use a different name.*`) | ||
|
||
// Creating an offer with a new name. | ||
results, err = client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "[email protected]", "test-offer-foo", "test offer description") | ||
c.Assert(err, gc.Equals, nil) | ||
c.Assert(results, gc.HasLen, 1) | ||
c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) | ||
} | ||
|
||
func (s *applicationOffersSuite) TestGetConsumeDetails(c *gc.C) { | ||
conn := s.open(c, nil, "[email protected]") | ||
defer conn.Close() | ||
|
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 guess again can we have the gorm tag too plz
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.
Will add this in a follow-up