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

expose AddError via Dao interface #744

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Conversation

YasserRabee
Copy link
Contributor

  • Do only one thing
  • Non breaking API changes
  • Tested

Note: test suite pass but didn't add a new test. Tested the change in real application though.

What did this pull request do?

Expose DB.AddError() to DAO interface.

User Case Description

We build most of our queries using reusable Scopes() functions. In most cases the query validates few constraints and needs a way to propagate the error back.

@jinzhu
Copy link
Member

jinzhu commented May 10, 2023

Seems the error is not added to the current db but initialized a new one?

@YasserRabee
Copy link
Contributor Author

Seems the error is not added to the current db but initialized a new one?

Thanks @jinzhu for replying

I opted to reuse the existing withError method that seems to be everywhere else. What do you think?

do.go Outdated
@@ -835,6 +835,10 @@ func (d *DO) newResultSlicePointer() interface{} {
return reflect.New(reflect.SliceOf(reflect.PtrTo(d.modelType))).Interface()
}

func (d *DO) AddError(err error) error {
return d.withError(err).underlyingDB().Error
Copy link
Member

@jinzhu jinzhu May 24, 2023

Choose a reason for hiding this comment

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

maybe change the code to

d.underlyingDB().AddError(err)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jinzhu.

I have update the PR and noticed the old implementation won't propagate the error back correctly.

@jinzhu jinzhu merged commit c229eb9 into go-gorm:master Jul 4, 2023
jeepc pushed a commit to jeepc/gen that referenced this pull request Nov 20, 2023
* expose AddError via Dao interface

* fix: mutate existing db to propagate the error back
@AmmarRabie AmmarRabie deleted the dao_add_error branch December 12, 2023 06:35
@AmmarRabie AmmarRabie restored the dao_add_error branch December 12, 2023 06:35
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.

2 participants