Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

For nulls.Time, decode empty value as NULL #2359

Merged
merged 1 commit into from
Jan 22, 2023

Conversation

travisturner
Copy link
Contributor

What is being done in this PR?

This PR adds a check during decoding which will return nulls.Time as a NULL value (i.e. its value for Valid will be false). Without this check, the value of nulls.Time{} will be:

{
  Time: 0001-01-01 00:00:00 +0000 UTC,
  Valid: true,
}

This ends up storing the value 0001-01-01 00:00:00 +0000 UTC in the database, as opposed to a NULL value.

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

One option was to make the model Bindable and write a custom Bind() method which could handle empty values, but the default Bind handler does so much that bypassing it seemed extreme.

What was discovered while working on it? (Optional)

Type in here a description of the discoveries made while working on this PR.

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

Fizz (using Postgres):

create_table("people") {
	t.Column("id", "uuid", {primary: true})
	t.Column("name", "string", {"size": 128})
	t.Column("birthday", "date", {"null": true})
	t.Timestamps()
}

Model:

type Person struct {
	ID uuid.UUID `json:"id" db:"id"`
	Name  string `json:"name" db:"name"`
	Birthday nulls.Time `json:"birthday" db:"birthday"`
	CreatedAt time.Time `json:"created_at" db:"created_at"`
	UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
}

Plush form:

<%= f.InputTag("Birthday", {"type":"date"}) %>

This commit adds a check during decoding which will return `nulls.Time` as
a NULL value (i.e. its value for `Valid` will be false). Without this
check, the value of nulls.Time{} will be:
```
{
  Time: 0001-01-01 00:00:00 +0000 UTC,
  Valid: true,
}
```
This ends up storing the value `0001-01-01 00:00:00 +0000 UTC` in the
database, as opposed to a NULL value.
@travisturner travisturner requested a review from a team as a code owner January 3, 2023 20:28
@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Jan 10, 2023
Copy link
Member

@sio4 sio4 left a comment

Choose a reason for hiding this comment

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

LGTM!

@sio4 sio4 self-assigned this Jan 22, 2023
@sio4 sio4 added bug Something isn't working s: accepted was accepted or confirmed and removed s: triage Some tests need to be run to confirm the issue labels Jan 22, 2023
@sio4 sio4 added this to the v1.1.0 milestone Jan 22, 2023
@sio4 sio4 merged commit e3d3a01 into gobuffalo:v1 Jan 22, 2023
@travisturner travisturner deleted the tlt/empty-null-time branch January 22, 2023 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working s: accepted was accepted or confirmed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants