Skip to content

Commit

Permalink
satellite/satellitedb: remove references to last_verification_reminder
Browse files Browse the repository at this point in the history 
Removing all references to column last_verification_reminder which is to be removed, due to new column verification_reminders

Issue: #4560

Change-Id: I7c9a426e946c7aed58e62c1eef80629daf6b1272
  • Loading branch information
@lizzythomson
lizzythomson committed Jan 5, 2023
1 parent 1d4411f commit 591615b
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 231 deletions.
5 changes: 1 addition & 4 deletions satellite/console/users.go
View file
Expand Up @@ -182,8 +182,7 @@ type User struct {

SignupPromoCode string `json:"signupPromoCode"`

LastVerificationReminder time.Time `json:"lastVerificationReminder"`
VerificationReminders int `json:"verificationReminders"`
VerificationReminders int `json:"verificationReminders"`

FailedLoginCount int `json:"failedLoginCount"`
LoginLockoutExpiration time.Time `json:"loginLockoutExpiration"`
Expand Down Expand Up @@ -249,8 +248,6 @@ type UpdateUserRequest struct {
MFASecretKey **string
MFARecoveryCodes *[]string

LastVerificationReminder **time.Time

// failed_login_count is nullable, but we don't really have a reason
// to set it to NULL, so it doesn't need to be a double pointer here.
FailedLoginCount *int
Expand Down
47 changes: 19 additions & 28 deletions satellite/console/users_test.go
View file
Expand Up @@ -197,7 +197,6 @@ func testUsers(ctx context.Context, t *testing.T, repository console.Users, user
assert.False(t, user.MFAEnabled)
assert.Empty(t, user.MFASecretKey)
assert.Empty(t, user.MFARecoveryCodes)
assert.Empty(t, user.LastVerificationReminder)

if user.IsProfessional {
assert.Equal(t, workingOn, userByEmail.WorkingOn)
Expand All @@ -220,7 +219,6 @@ func testUsers(ctx context.Context, t *testing.T, repository console.Users, user
assert.False(t, user.MFAEnabled)
assert.Empty(t, user.MFASecretKey)
assert.Empty(t, user.MFARecoveryCodes)
assert.Empty(t, user.LastVerificationReminder)

if user.IsProfessional {
assert.Equal(t, workingOn, userByID.WorkingOn)
Expand Down Expand Up @@ -253,38 +251,32 @@ func testUsers(ctx context.Context, t *testing.T, repository console.Users, user
oldUser, err := repository.GetByEmail(ctx, email)
assert.NoError(t, err)

d := (60 * time.Second)
date := time.Now().Add(-24 * 365 * time.Hour).Truncate(d)

newUserInfo := &console.User{
ID: oldUser.ID,
FullName: newName,
ShortName: newLastName,
Email: newEmail,
Status: console.Active,
PaidTier: true,
MFAEnabled: true,
MFASecretKey: mfaSecretKey,
MFARecoveryCodes: []string{"1", "2"},
PasswordHash: []byte(newPass),
LastVerificationReminder: date,
ID: oldUser.ID,
FullName: newName,
ShortName: newLastName,
Email: newEmail,
Status: console.Active,
PaidTier: true,
MFAEnabled: true,
MFASecretKey: mfaSecretKey,
MFARecoveryCodes: []string{"1", "2"},
PasswordHash: []byte(newPass),
}

shortNamePtr := &newUserInfo.ShortName
secretKeyPtr := &newUserInfo.MFASecretKey
lastVerificationReminderPtr := &newUserInfo.LastVerificationReminder

err = repository.Update(ctx, newUserInfo.ID, console.UpdateUserRequest{
FullName: &newUserInfo.FullName,
ShortName: &shortNamePtr,
Email: &newUserInfo.Email,
Status: &newUserInfo.Status,
PaidTier: &newUserInfo.PaidTier,
MFAEnabled: &newUserInfo.MFAEnabled,
MFASecretKey: &secretKeyPtr,
MFARecoveryCodes: &newUserInfo.MFARecoveryCodes,
PasswordHash: newUserInfo.PasswordHash,
LastVerificationReminder: &lastVerificationReminderPtr,
FullName: &newUserInfo.FullName,
ShortName: &shortNamePtr,
Email: &newUserInfo.Email,
Status: &newUserInfo.Status,
PaidTier: &newUserInfo.PaidTier,
MFAEnabled: &newUserInfo.MFAEnabled,
MFASecretKey: &secretKeyPtr,
MFARecoveryCodes: &newUserInfo.MFARecoveryCodes,
PasswordHash: newUserInfo.PasswordHash,
})
assert.NoError(t, err)

Expand All @@ -299,7 +291,6 @@ func testUsers(ctx context.Context, t *testing.T, repository console.Users, user
assert.True(t, newUser.MFAEnabled)
assert.Equal(t, mfaSecretKey, newUser.MFASecretKey)
assert.Equal(t, newUserInfo.MFARecoveryCodes, newUser.MFARecoveryCodes)
assert.Equal(t, newUserInfo.LastVerificationReminder, newUser.LastVerificationReminder)
// PartnerID should not change
assert.Equal(t, user.PartnerID, newUser.PartnerID)
assert.Equal(t, oldUser.CreatedAt, newUser.CreatedAt)
Expand Down
1 change: 0 additions & 1 deletion satellite/satellitedb/dbx/satellitedb.dbx
View file
Expand Up @@ -407,7 +407,6 @@ model user (

field signup_promo_code text ( updatable, nullable )

field last_verification_reminder timestamp ( updatable, nullable )
field verification_reminders int ( updatable, default 0 )

field failed_login_count int ( updatable, nullable )
Expand Down
259 changes: 104 additions & 155 deletions satellite/satellitedb/dbx/satellitedb.dbx.go
View file

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion satellite/satellitedb/dbx/satellitedb.dbx.pgx.sql
View file
Expand Up @@ -503,7 +503,6 @@ CREATE TABLE users (
mfa_secret_key text,
mfa_recovery_codes text,
signup_promo_code text,
last_verification_reminder timestamp with time zone,
verification_reminders integer NOT NULL DEFAULT 0,
failed_login_count integer,
login_lockout_expiration timestamp with time zone,
Expand Down
1 change: 0 additions & 1 deletion satellite/satellitedb/dbx/satellitedb.dbx.pgxcockroach.sql
View file
Expand Up @@ -503,7 +503,6 @@ CREATE TABLE users (
mfa_secret_key text,
mfa_recovery_codes text,
signup_promo_code text,
last_verification_reminder timestamp with time zone,
verification_reminders integer NOT NULL DEFAULT 0,
failed_login_count integer,
login_lockout_expiration timestamp with time zone,
Expand Down
6 changes: 6 additions & 0 deletions satellite/satellitedb/migrate_test.go
View file
Expand Up @@ -296,6 +296,12 @@ func migrateTest(t *testing.T, connStr string) {
conversionRates.RemoveColumn("rate_gob")
}

// TODO(lizzy): remove this check with the migration step to drop the column last_verification_reminders.
users, ok := finalSchema.FindTable("users")
if ok {
users.RemoveColumn("last_verification_reminder")
}

// verify that we also match the dbx version
require.Equal(t, dbxschema, finalSchema, "result of all migration scripts did not match dbx schema")
}
Expand Down
11 changes: 0 additions & 11 deletions satellite/satellitedb/users.go
View file
Expand Up @@ -351,13 +351,6 @@ func toUpdateUser(request console.UpdateUserRequest) (*dbx.User_Update_Fields, e
update.MfaRecoveryCodes = dbx.User_MfaRecoveryCodes(string(recoveryBytes))
}
}
if request.LastVerificationReminder != nil {
if *request.LastVerificationReminder == nil {
update.LastVerificationReminder = dbx.User_LastVerificationReminder_Null()
} else {
update.LastVerificationReminder = dbx.User_LastVerificationReminder(**request.LastVerificationReminder)
}
}
if request.FailedLoginCount != nil {
update.FailedLoginCount = dbx.User_FailedLoginCount(*request.FailedLoginCount)
}
Expand Down Expand Up @@ -454,10 +447,6 @@ func userFromDBX(ctx context.Context, user *dbx.User) (_ *console.User, err erro
result.SignupPromoCode = *user.SignupPromoCode
}

if user.LastVerificationReminder != nil {
result.LastVerificationReminder = *user.LastVerificationReminder
}

if user.FailedLoginCount != nil {
result.FailedLoginCount = *user.FailedLoginCount
}
Expand Down
43 changes: 13 additions & 30 deletions satellite/satellitedb/users_test.go
View file
Expand Up @@ -70,20 +70,19 @@ func TestUpdateUser(t *testing.T) {
require.NoError(t, err)

newInfo := console.User{
FullName: "updatedFullName",
ShortName: "updatedShortName",
PasswordHash: []byte("updatedPasswordHash"),
ProjectLimit: 1,
ProjectBandwidthLimit: 1,
ProjectStorageLimit: 1,
ProjectSegmentLimit: 1,
PaidTier: true,
MFAEnabled: true,
MFASecretKey: "secretKey",
MFARecoveryCodes: []string{"code1", "code2"},
LastVerificationReminder: time.Now().Truncate(time.Second),
FailedLoginCount: 1,
LoginLockoutExpiration: time.Now().Truncate(time.Second),
FullName: "updatedFullName",
ShortName: "updatedShortName",
PasswordHash: []byte("updatedPasswordHash"),
ProjectLimit: 1,
ProjectBandwidthLimit: 1,
ProjectStorageLimit: 1,
ProjectSegmentLimit: 1,
PaidTier: true,
MFAEnabled: true,
MFASecretKey: "secretKey",
MFARecoveryCodes: []string{"code1", "code2"},
FailedLoginCount: 1,
LoginLockoutExpiration: time.Now().Truncate(time.Second),
}

require.NotEqual(t, u.FullName, newInfo.FullName)
Expand All @@ -97,7 +96,6 @@ func TestUpdateUser(t *testing.T) {
require.NotEqual(t, u.MFAEnabled, newInfo.MFAEnabled)
require.NotEqual(t, u.MFASecretKey, newInfo.MFASecretKey)
require.NotEqual(t, u.MFARecoveryCodes, newInfo.MFARecoveryCodes)
require.NotEqual(t, u.LastVerificationReminder, newInfo.LastVerificationReminder)
require.NotEqual(t, u.FailedLoginCount, newInfo.FailedLoginCount)
require.NotEqual(t, u.LoginLockoutExpiration, newInfo.LoginLockoutExpiration)

Expand Down Expand Up @@ -257,21 +255,6 @@ func TestUpdateUser(t *testing.T) {
u.MFARecoveryCodes = newInfo.MFARecoveryCodes
require.Equal(t, u, updatedUser)

// update just last verification reminder
lastReminderPtr := &newInfo.LastVerificationReminder
updateReq = console.UpdateUserRequest{
LastVerificationReminder: &lastReminderPtr,
}

err = users.Update(ctx, id, updateReq)
require.NoError(t, err)

updatedUser, err = users.Get(ctx, id)
require.NoError(t, err)

u.LastVerificationReminder = newInfo.LastVerificationReminder
require.Equal(t, u, updatedUser)

// update just failed login count
updateReq = console.UpdateUserRequest{
FailedLoginCount: &newInfo.FailedLoginCount,
Expand Down

0 comments on commit 591615b

Please sign in to comment.