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

fixed the bug in the extra args handling #93

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Conversation

sio4
Copy link
Member

@sio4 sio4 commented Feb 4, 2023

What is being done in this PR?

Currently, the type of all extra columns is set as string which is the default and the null handling is not working. With this fix,

  • the type for the extra fields is preserved and properly applied to both the model and migration file
  • if the type is nullable, a form of nulls.Type supported by module nulls, the migration will allow the null value.

Now the generated files are something like

type User struct {
	ID           uuid.UUID `json:"id" db:"id"`
	CreatedAt    time.Time `json:"created_at" db:"created_at"`
	UpdatedAt    time.Time `json:"updated_at" db:"updated_at"`
	Email        string    `json:"email" db:"email"`
	PasswordHash string    `json:"password_hash" db:"password_hash"`

	Password             string `json:"-" db:"-"`
	PasswordConfirmation string `json:"-" db:"-"`

	//Extra fields
	Age int          `json:"age" db:"age"`
	Bio nulls.String `json:"bio" db:"bio"`
}
create_table("users"){
    t.Column("id", "uuid", {"primary": true})
    t.Column("email", "string", {})
    t.Column("password_hash", "string", {})
    t.Column("age", "integer", {})
    t.Column("bio", "text", {"null": true})
}

for the same command used in #21, buffalo g auth age:int bio:nulls.Text.

fixes #21

What are the main choices made to get to this solution?

Actually, there could be better options such as integration with fizz, and improving the function and the logic used for extra arguments handling, but I just simply fixed the bug with a smaller modification while keeping the current implementation as is today.

List the manual test cases you've covered before sending this PR:

Manual test with a simple todo app.

@sio4 sio4 added this to the v1.5.0 milestone Feb 4, 2023
@sio4 sio4 requested a review from a team February 4, 2023 16:26
@sio4 sio4 self-assigned this Feb 4, 2023
@sio4 sio4 merged commit 931921b into main Feb 6, 2023
@sio4 sio4 deleted the fixing-extra-args-handling branch February 6, 2023 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

attr type didn't work
1 participant