forcedotcom / go-soql

Golang tag library to generate SOQL queries
BSD 3-Clause "New" or "Revised" License
52 stars 12 forks source link

go-soql: Add Date type #34

Closed mskwon closed 2 years ago

mskwon commented 2 years ago

resolves #33

salesforce-cla[bot] commented 2 years ago

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Michael Kwon m***@a***.com. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce.com Contributor License Agreement and this Pull Request will be revalidated.

salesforce-cla[bot] commented 2 years ago

Thanks for the contribution! Before we can merge this, we need @mskwon to sign the Salesforce.com Contributor License Agreement.

atulkc commented 2 years ago

@mskwon Thanks for submitting this PR. However, I think introducing a custom type specifically for only this library will not play well with other tag libraries like json and yaml. Also, all that is being done as part of this new type is use different parsing format. Here's my proposal to keep it generic and make it extensible in future:

What you really want is the way to declaratively specify date format. I think we should introduce a new optional parameter named format that can be added to field of type time.Time and use that to specify the right date time format. We can default to existing DateTimeFormat if none is provided. We will also need to make sure that this parameter is only added for time.Time fields. So the tags should look like this:

type QueryCriteriaWithPtrDateTimeType struct {
        // Makes CreatedDate to be parsed in DateFormat
    CreatedDate  *time.Time `soql:"equalsOperator,fieldName=CreatedDate,format=DateFormat"`
        // ResolvedDate will be parsed in default DateTimeFormat
    ResolvedDate *time.Time `soql:"equalsOperator,fieldName=ResolvedDate"`
}

This allows us to use standard go data types and allow users to control the format. We can add support for different formats later by adding constants like DateFormat, DateTimeFormat...etc.

Thoughts?

mskwon commented 2 years ago

That sounds like a much better idea - though I was looking at the clauseBuilderMap and noticed that the current function signature doesn't really allow for tag data to be passed through right now...

How should I approach this?

atulkc commented 2 years ago

We will have to change the function signature in clauseBuilderMap to take a map[string]string to represent any additional parameters. All the functions need to be updated to take this map and use it wherever applicable (in this case when formatting time field).

var clauseBuilderMap = map[string]func(v interface{}, fieldName string, map[string]string) (string, error){

In func marshalWhereClause create a new map with all parameters name value pairs. Introduce a new function

func getTagParameterMap(clauseTag string) map[string]string {
// Here do same thing as getTagValue function but accumulate all parameters with `=` in a map with left hand as key and right hand as value
}

Invoke this new function in marshalWhereClause and then use the map returned by this map in invoking the function you look up from clauseBuilderMap

mskwon commented 2 years ago

Got it, how about this?