split update methods into two for offer data (#2133)

* change Offers interface to separate Update method into two.

* Implement Finish and Redeem method to avoid concurrent updates

* Implement FinishOffer and RedeemOffer service methods

* add tests

* fix linting issue

* add tests for checking Finish and Redeem's results to work as expected

* fix linting error

satellite/marketing/offers.go

@@ -18,7 +18,8 @@ type Offers interface {
 	ListAll(ctx context.Context) ([]Offer, error)
 	GetCurrentByType(ctx context.Context, offerType OfferType) (*Offer, error)
 	Create(ctx context.Context, offer *NewOffer) (*Offer, error)
-	Update(ctx context.Context, offer *UpdateOffer) error
+	Redeem(ctx context.Context, offerID int) error
+	Finish(ctx context.Context, offerID int) error
 }
 
 // NewOffer holds information that's needed for creating a new offer
@@ -42,10 +43,9 @@ type NewOffer struct {
 
 // UpdateOffer holds fields needed for update an offer
 type UpdateOffer struct {
-	ID          int
-	Status      OfferStatus
-	NumRedeemed int
-	ExpiresAt   time.Time
+	ID        int
+	Status    OfferStatus
+	ExpiresAt time.Time
 }
 
 // OfferType indicates the type of an offer

satellite/marketing/offers_test.go

@@ -59,13 +59,25 @@ func TestOffer_Database(t *testing.T) {
 			require.Equal(t, new, c)
 
 			update := &marketing.UpdateOffer{
-				ID:          new.ID,
-				Status:      marketing.Done,
-				NumRedeemed: new.NumRedeemed,
-				ExpiresAt:   time.Now(),
+				ID:        new.ID,
+				Status:    marketing.Done,
+				ExpiresAt: time.Now(),
 			}
-			err = planet.Satellites[0].DB.Marketing().Offers().Update(ctx, update)
+
+			err = planet.Satellites[0].DB.Marketing().Offers().Redeem(ctx, update.ID)
 			require.NoError(t, err)
+
+			err = planet.Satellites[0].DB.Marketing().Offers().Finish(ctx, update.ID)
+			require.NoError(t, err)
+
+			current, err := planet.Satellites[0].DB.Marketing().Offers().ListAll(ctx)
+			require.NoError(t, err)
+			if new.Status == marketing.Default {
+				require.Equal(t, new.NumRedeemed, current[i].NumRedeemed)
+			} else {
+				require.Equal(t, new.NumRedeemed+1, current[i].NumRedeemed)
+			}
+			require.Equal(t, marketing.Done, current[i].Status)
 		}
 
 		// create with expired offer

satellite/marketing/service.go

@@ -53,7 +53,7 @@ func (s *Service) ListAllOffers(ctx context.Context) (offers []Offer, err error)
 func (s *Service) GetCurrentOfferByType(ctx context.Context, offerType OfferType) (offer *Offer, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	offer, err = s.db.Offers().GetCurrentByType(ctx, offer.Type)
+	offer, err = s.db.Offers().GetCurrentByType(ctx, offerType)
 	if err != nil {
 		return nil, Error.Wrap(err)
 	}
@@ -78,14 +78,27 @@ func (s *Service) InsertNewOffer(ctx context.Context, offer *NewOffer) (o *Offer
 	return o, nil
 }
 
-// UpdateOffer modifies an existing offer in the db when the offer status is set to NoStatus
-func (s *Service) UpdateOffer(ctx context.Context, offer *UpdateOffer) (err error) {
+// RedeemOffer adds 1 to the number of redeemed for an offer
+func (s *Service) RedeemOffer(ctx context.Context, uo *UpdateOffer) (err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	if offer.Status == Default {
-		offer.NumRedeemed = 0
+	if uo.Status == Default {
+		return nil
 	}
-	err = s.db.Offers().Update(ctx, offer)
+
+	err = s.db.Offers().Redeem(ctx, uo.ID)
+	if err != nil {
+		return Error.Wrap(err)
+	}
+
+	return nil
+}
+
+// FinishOffer updates an active offer's status to be Done and its expiration time to be now
+func (s *Service) FinishOffer(ctx context.Context, oID int) (err error) {
+	defer mon.Task()(&ctx)(&err)
+
+	err = s.db.Offers().Finish(ctx, oID)
 	if err != nil {
 		return Error.Wrap(err)
 	}

satellite/satellitedb/locked.go

@@ -653,6 +653,12 @@ func (m *lockedOffers) Create(ctx context.Context, offer *marketing.NewOffer) (*
 	return m.db.Create(ctx, offer)
 }
 
+func (m *lockedOffers) Finish(ctx context.Context, offerId int) error {
+	m.Lock()
+	defer m.Unlock()
+	return m.db.Finish(ctx, offerId)
+}
+
 func (m *lockedOffers) GetCurrentByType(ctx context.Context, offerType marketing.OfferType) (*marketing.Offer, error) {
 	m.Lock()
 	defer m.Unlock()
@@ -665,10 +671,10 @@ func (m *lockedOffers) ListAll(ctx context.Context) ([]marketing.Offer, error) {
 	return m.db.ListAll(ctx)
 }
 
-func (m *lockedOffers) Update(ctx context.Context, offer *marketing.UpdateOffer) error {
+func (m *lockedOffers) Redeem(ctx context.Context, offerId int) error {
 	m.Lock()
 	defer m.Unlock()
-	return m.db.Update(ctx, offer)
+	return m.db.Redeem(ctx, offerId)
 }
 
 // Orders returns database for orders

satellite/satellitedb/offers.go

@@ -104,15 +104,28 @@ func (offers *offers) Create(ctx context.Context, o *marketing.NewOffer) (*marke
 	return newOffer, marketing.OffersErr.Wrap(tx.Commit())
 }
 
-// Update modifies an offer entry's status and amount of offers redeemed based on offer id
-func (offers *offers) Update(ctx context.Context, o *marketing.UpdateOffer) error {
-	updateFields := dbx.Offer_Update_Fields{
-		Status:      dbx.Offer_Status(int(o.Status)),
-		NumRedeemed: dbx.Offer_NumRedeemed(o.NumRedeemed),
-		ExpiresAt:   dbx.Offer_ExpiresAt(o.ExpiresAt),
+// Redeem adds 1 to the amount of offers redeemed based on offer id
+func (offers *offers) Redeem(ctx context.Context, oID int) error {
+	statement := offers.db.Rebind(
+		`UPDATE offers SET num_redeemed = num_redeemed + 1 where id = ? AND status = ? AND num_redeemed < redeemable_cap`,
+	)
+
+	_, err := offers.db.DB.ExecContext(ctx, statement, oID, marketing.Active)
+	if err != nil {
+		return marketing.OffersErr.Wrap(err)
 	}
 
-	offerID := dbx.Offer_Id(o.ID)
+	return nil
+}
+
+// Finish changes the offer status to be Done and its expiration date to be now based on offer id
+func (offers *offers) Finish(ctx context.Context, oID int) error {
+	updateFields := dbx.Offer_Update_Fields{
+		Status:    dbx.Offer_Status(int(marketing.Done)),
+		ExpiresAt: dbx.Offer_ExpiresAt(time.Now().UTC()),
+	}
+
+	offerID := dbx.Offer_Id(oID)
 
 	_, err := offers.db.Update_Offer_By_Id(ctx, offerID, updateFields)
 	if err != nil {