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

time.Time conversion generates boilergql.TimeTimeToTime() #7

Closed
troian opened this issue May 20, 2020 · 28 comments
Closed

time.Time conversion generates boilergql.TimeTimeToTime() #7

troian opened this issue May 20, 2020 · 28 comments
Labels
bug Something isn't working

Comments

@troian
Copy link
Collaborator

troian commented May 20, 2020

		ID:        m.ID,
		StartsAt:  boilergql.TimeTimeToTime(m.StartsAt),
		EndsAt:    boilergql.TimeTimeToTime(m.EndsAt),

expected

		ID:        m.ID,
		StartsAt:  m.StartsAt,
		EndsAt:    m.EndsAt,
@RichardLindhout
Copy link
Member

Are both times exactly the same type?

@RichardLindhout
Copy link
Member

Based on TimeTimeToTime they are

@RichardLindhout
Copy link
Member

Strange, looking into it!

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

Yah, types are equal

@RichardLindhout
Copy link
Member

It should skip these as custom convert since type should be the same here
https://github.com/web-ridge/gqlgen-sqlboiler/blob/master/convert.go#L814

Just to be sure I did not know gqglen supported go timestamps when will they be generated?

@RichardLindhout
Copy link
Member

So I can test if it's fixed. It generates as Int for me a.t.m. maybe we should change in sqlboiler-graphql-schema too.

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

Thats what I have in gqlgen.yml

models:
  GeoPoint:
    model:
      - github.com/web-ridge/utils-go/boilergql.GeoPoint
  DateTime:
    model:
      - github.com/99designs/gqlgen/graphql.Time

and graphql.schema uses them like

scalar GeoPoint
scalar DateTime

type Type {
    startsAt: DateTime
    endsAt: DateTime
    createdAt: DateTime
    updatedAt: DateTime
}

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

gqlgen supports time.Time https://github.com/99designs/gqlgen/blob/master/graphql/time.go

@RichardLindhout RichardLindhout added the bug Something isn't working label May 20, 2020
@RichardLindhout
Copy link
Member

Ok, I'm working on createdAt, updatedAt too a.t.m. will hopefully be able to reproduce this bug this evening and release a new version!

@RichardLindhout
Copy link
Member

Maybe you can speedup this process I need more types to ignore in input but multiple ignores like this do not work https://github.com/web-ridge/sqlboiler-graphql-schema
--skip-input-fields=organizationId,createdAt,updatedAt

have you used multiple ignores yet?

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

I haven't. Going to try

@RichardLindhout
Copy link
Member

I found it

@RichardLindhout
Copy link
Member

Needs to be this I'll update documentation for that and going on with the timestamp!

@RichardLindhout
Copy link
Member

<-- forgot cli snippet: Needs to be this --skip-input-fields=organizationId --skip-input-fields=createdAt --skip-input-fields=updatedAt

I'm up and running but I need to add some extra changes since timestamps are nullable in database a.t.m.

@RichardLindhout
Copy link
Member

Interesting it should return time.Time but it returns *Time which is the root problem
Schermafbeelding 2020-05-20 om 20 56 49

@RichardLindhout
Copy link
Member

Root cause discovered adding fix + test to resolve this
Schermafbeelding 2020-05-20 om 21 15 33

@RichardLindhout
Copy link
Member

One side effect we need to update this repo with also TimeTime on grapql side. It looks a little strange but it will work with future packages too

@RichardLindhout
Copy link
Member

@RichardLindhout
Copy link
Member

You also need to update this package since there will be changes to conversion functions
-->
fca24e7

@RichardLindhout
Copy link
Member

So updating to

github.com/web-ridge/[email protected]
github.com/web-ridge/[email protected]

Should fix this bug

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

Thats with the latest release. Time.TimeToMods

	queryMods = append(queryMods, Time.TimeToMods(m.StartsAt, models.AppointmentParticipantColumns.StartsAt)...)
	queryMods = append(queryMods, Time.TimeToMods(m.EndsAt, models.AppointmentParticipantColumns.EndsAt)...)

@RichardLindhout
Copy link
Member

Oh my bad.. will fix this.

You have TimeFilter in your schema added then or not?

It will become TimeDotTimeToMods

@RichardLindhout
Copy link
Member

@troian
Copy link
Collaborator Author

troian commented May 20, 2020

TimeDotTimeToMods not generated

	queryMods = append(queryMods, TimeDotTimeToMods(m.StartsAt, models.AppointmentParticipantColumns.StartsAt)...)
	queryMods = append(queryMods, TimeDotTimeToMods(m.EndsAt, models.AppointmentParticipantColumns.EndsAt)...)

@RichardLindhout
Copy link
Member

Please add TimeDotTimeToMods add it as a filter_custom.go in your helpers package like this we need to do more work to support all gqlgen scalars and/or custom scalars + enums in filtering.

It need some more polishing and I don't think I have time this month but will invest more next month in new features. I'm not sure this will be first but I think also in improving merging or overriding functions and adding support for multiple schema.graphql so you can opt-out for some schema's not sure yet.

We want to support this so I created some issues in both repositories to improve the custom scalars
web-ridge/sqlboiler-graphql-schema#7
web-ridge/gqlgen-sqlboiler#28

@RichardLindhout
Copy link
Member

Like this it should work, let me know if it does not.

Schermafbeelding 2020-05-20 om 22 26 05

@RichardLindhout
Copy link
Member

filter_custom.go so it will not be overridden when you convert again ;)

@RichardLindhout
Copy link
Member

Closing in favor of the issues
web-ridge/sqlboiler-graphql-schema#7
web-ridge/gqlgen-sqlboiler#28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants