Adds build method - helper to get types

[Xosmond]

Hello! Thanks for maintaining Avram!

I wanted to generate a named couple to send it as params so I had to write a macro for that, also I had to define the params types to I created an alias for that, also I wanted to pass attributes overwriting so I modify some methods.

Is there a different way to do this? Is this code useful for you? Let me know what do you think. I have been working with this together with Minitest.cr and works smoothly 👍

[paulcsmith]

Hey @Xosmond! This is definitely beneficial for stuff like testing params in operations or JSON APIs, so this is very welcome. I think with a couple changes we can merge this!

• Rename build to named_tuple. build in other factory libraries means building an object without saving. I'd like to leave that method open for Add Box#build #34. I think named_tuple is a bit long, so I'm open to something else! Maybe params? User.params
• Add params(block) so that you can customize it like you would create with a block
• Specs for added/changed methods

I'm not sure about allowing passing in a NamedTuple to create. I think I'd rather remove that and make it so customization only happens with the block form and methods. Is there a use-case for passing a NamedTuple instead?

Thanks for your work on this!

[Xosmond]

Hey @paulcsmith

Sorry for the bad description of the PR

This is a summary with some examples:

1. Params and Types
You can use this to send it as params to an endpoint or you can serialize it to json for other purposes on testing

params = SomeBox.params

Here you can use PARAMS_TYPES to define the types when needed, useful in some contexts (for example: when working with minitest.cr)

@params : SomeBox::PARAMS_TYPES = SomeBox.params

2. Params, create, new receive a named_tuple to overwrite attributes easily

# Using a block would be big and hard to read
params  = SomeBox.params do |box|
  box.name = "overwrite name"
  box.other = "overwrite other"
end

# I don't really like this way either
SomeBox.params(&.name("not-a-sequence"))

# Sending a named_tuple would be clean and easy to read
params = SomeBox.params({ name: "overwrite name", other: "overwrite other"})
some = SomeBox.create({ name: "overwrite name", other: "overwrite other"})

[jwoertink]

Just looking over this, and I'm sort of confused on what this is doing exactly. It looks like this params method is creating a new instance of the object, assigning the attributes, then just returning those attributes? I think the name params is throwing me off here because when I see that word, I instantly picture foo=bar&baz=qux. Also having it as a class method seems like it's going to return some object at the class level (as opposed to doing SomeBox.new.params or whatever).

The other thing I'm wondering about is I see that this PARAMS_TYPES constant makes everything nilable. Wouldn't we run in to type-safety issues if we created this NamedTuple with nil fields on columns that aren't nilable, and then tried to use that in a SaveOperation?

If I'm understanding the intention here, you want to get the attributes of a model instance so you can easily craft params to send to say an Action for your tests, right?

# something like this?
tag = TagBox.create # with all the defaults defined in the box

SomeApi.post("/whatever", params: tag.attributes)

And then you want to be able to override the defaults?

tag = TagBox.create(&.title("Override"))

SomeApi.post("/whatever", params: tag.attributes)

If that's the case, then I think you can actually do that now using tag.operation.attributes. In which case, we could just add a simple helper method

def attributes
  operation.attributes
end

Now, I know this doesn't solve the issue of being able to define that instance variable for minitest, but maybe going this route would make that irrelevant?

I think the only other thing might be that TagBox.create will save a record to the database, but maybe you don't want that to happen at this level? In which case, what we really need is #34 like @paulcsmith mentioned.

I do want to reiterate that I'm sort of foggy on this, so please if I'm misunderstanding the intention, let me know. I'm all for making things easier to use with minitest since I know not everyone will want to use the built-in specs. I just want to make sure this is clear!

Thanks!

[jwoertink]

Just noticed #371 which I think this PR would close. What if this method was called UserBox.build_attributes? A name like that might make it really clear on what the intention is, and what you'd expect it to return.

[Xosmond]

Helo @jwoertink . Well yea, the idea behind this, is to generate the attributes for sending the params, before actually creating something, because when testing you want to isolate things, you don’t want to create records on database that would lead on random errors on testing.

I agree with build_attributes Would make more sense.

About the types PARAMS_TYPES, the reason behind this, is that in some scenarios we are going to need the types for defining a variable. I put as an example minitest but in general that would be useful.

Don’t you think that Tagbox.create({title: “Override”}) would be more readable thanTagBox.create(&.title("Override")) and even more when more attributes are overridden.

Also using a NamedTuple Allows you to reuse that in multiple places when testing.

My desire on this PR was just suggest some extra built in features that would allow to write better tests. If you think this will be covered on #34, all good, I just can close this :)

I appreciate your time for checking this

Best

[jwoertink]

Hey @Xosmond thanks for the explanation! I'm on board with this change now. I think the rename from params to build_attributes would be good.

As for the create(&.title()), this helps to follow a core ideal to the Lucky framework to solidify the "type-safety" that we strive for. Now granted, maybe it doesn't look as nice at first, but once you've been around Lucky enough, you'll see that this same pattern is used in lots of areas like TagQuery.new.distinct_on(&.title).title("Happy Cow").or(&.title("Angry Duck")) (the "or" is coming soon).

I think #34 will actually cover a different aspect of a build method that will return an instance of the model with the attributes built, but the record not persisted. So I think we should keep this PR going. I don't think it's too far off. Now that I see the goal a bit better, I'll go through the whole thing and leave some notes as well.

Thanks for submitting this!

[paulcsmith]

I love the build_attributes name instead of params! I think we should not allow using a NamedTuplle for create/build_attributes. In part because we tend to yield blocks this, but mostly because the NamedTuple has really hard to decipher errors when you get the type wrong (it shows overloads for ever arg), but also because the NamedTuple version does not allow using other methods on the Box.

For example, if I wanted to have a fullname method that sets the name and also sets the email address, I could do so with methods and a yielded block, but would not work with the NamedTuple version:

class UserBox
  def initialize
    name("Someone")
    email("[email protected]")
  end

  def name=(value)
     name value
     email "#{value}@example.com"
  end
end

UserBox.create(&.name("Paul"))

This is a trivial example, but there are more complex ones where you may want to set a number of attributes with one method, or create other objects necessary to build the Box.

Maybe we can add NamedTupe to initialize in the future but for now I think it is best to stick with just the change that adds build_attributes and returns a NamedTuple and uses the same block syntax as now: UserBox.build_attributes(&.name("Jane"))

[Xosmond]

I think NameTuple can be a separate discussion then :). I think using blocks for small and simple boxes is fine, but for NamedTuples would be pretty useful for testing big and complex applications and when to reuse and change constantly things.

I have created this #449 since I can't change the branch to merge here.

Thanks for your comments