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

Set method for group fields + tests #454

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeronimoPaganini
Copy link

@JeronimoPaganini JeronimoPaganini commented Sep 14, 2021

Since in current version I didn't find how to use similar way to setting values for fields marked like Group, I propose my small improvement.

Example:

try Galaxy
    .query(on: self.database)
    .set(\.$size.$km, to: oneLightYearInKm)
    .set(\.$size.$lightYear, to: 1)
    .update()

where $size is Group fields in a model

    @Group(key: "area")
    public var size: Size
public final class Size: Fields {
    @Field(key: "km")
    var km: Double
    
    @Field(key: "light_year")
    var lightYear: Double
    
    public init() { }
}

If I misunderstood something, please, let me know :)

@0xTim 0xTim requested a review from gwynne September 14, 2021 15:50
@gwynne gwynne requested a review from Joannis September 16, 2021 07:04
@gwynne
Copy link
Member

gwynne commented Sep 16, 2021

@Joannis I need to look at this implementation more closely in any event, but am interested in your input, specifically re: the test failures for Mongo integration where we're apparently hitting the "didn't fetch or set before accessing" codepath?

@JeronimoPaganini
Copy link
Author

Hi @gwynne , I guess you meant @JeronimoPaganini ,
I'm a bit confused how it happened, since I've reused the same method set, and I've added just +1 signature here:
https://github.com/vapor/fluent-kit/pull/454/files#diff-060bb4b838cade73b63ca17a0ce5a59743bcc45b4ba559f4673bb3a6f79b9726R40

So the plan was just supporting group kodepath (I've re-checked it in my project using Postgres, but didn't check how it works with Mongo).

Could it be, since mongo-kit doesn't work with group codepaths? (Sorry for the stupid question, I didn't work with mongo-kit yet)

@Joannis
Copy link
Member

Joannis commented Sep 16, 2021

@JeronimoPaganini no, she meant me. I'm maintaining all MongoDB related components.

@JeronimoPaganini
Copy link
Author

JeronimoPaganini commented Sep 16, 2021

@Joannis, Ah, got it. We have the same 1st letter nickname "J", and since I've prepared this PR, I thought it's just T9 :)

@Joannis Joannis removed their request for review August 12, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants