Skip to content

Commit

Permalink
Odoo 16 storage: Use correct contact filter, make list and get co…
Browse files Browse the repository at this point in the history 
…nsistent (#202)

Uses the new style invoice contact filter `category_id` -> `type: invoice`.

Use the same filters for `list` and `get` so we don't have any inconsistencies between the two methods. Since `create` and `update` also use the same underlying method this fixes them too.

Also adds the lost mockgen statement.
  • Loading branch information
@bastjan
bastjan authored Feb 7, 2024
1 parent 2778cd7 commit a8e87c3
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 61 deletions.
59 changes: 41 additions & 18 deletions apiserver/billing/odoostorage/odoo/odoo16/odoo16.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ import (
const VSHNAccountingContactNameKey = "billing.appuio.io/vshn-accounting-contact-name"

// Used to identify the accounting contact of a company.
const roleAccountCategory = 70
const companyCategory = 2
const invoiceType = "invoice"

// TODO(bastjan) test if still needed in odoo16
const companyCategory = 2

// Used to generate the UUID for the .metadata.uid field.
var metaUIDNamespace = uuid.MustParse("7550b1ae-7a2a-485e-a75d-6f931b2cd73f")

var roleAccountFilter = odooclient.NewCriterion("category_id", "in", []int{roleAccountCategory})
var activeFilter = odooclient.NewCriterion("active", "=", true)
var invoiceTypeFilter = odooclient.NewCriterion("type", "=", invoiceType)
var notInflightFilter = odooclient.NewCriterion("vshn_control_api_inflight", "=", false)
var mustInflightFilter = odooclient.NewCriterion("vshn_control_api_inflight", "!=", false)

Expand Down Expand Up @@ -97,8 +98,8 @@ type FailedRecordScrubber struct {
sessionCreator func(ctx context.Context) (Odoo16Client, error)
}

//go:generate go run go.uber.org/mock/mockgen -destination=./odoo16mock/$GOFILE -package odoo16mock . Odoo16Client
type Odoo16Client interface {
Read(string, []int64, *odooclient.Options, interface{}) error
Update(string, []int64, interface{}) error
FindResPartners(*odooclient.Criteria, *odooclient.Options) (*odooclient.ResPartners, error)
CreateResPartner(*odooclient.ResPartner) (int64, error)
Expand Down Expand Up @@ -127,28 +128,45 @@ func (s *Odoo16Storage) get(ctx context.Context, name string) (company odooclien
return odooclient.ResPartner{}, odooclient.ResPartner{}, err
}

u := []odooclient.ResPartner{}
err = session.Read(odooclient.ResPartnerModel, []int64{int64(id)}, fetchPartnerFieldOpts, &u)
accp, err := session.FindResPartners(
newValidInvoiceRecordCriteria().AddCriterion(odooclient.NewCriterion("id", "=", id)),
fetchPartnerFieldOpts)
if err != nil {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching accounting contact by ID: %w", err)
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("error fetching accounting contact %d: %w", id, err)
}
if accp == nil {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching accounting contact %d returned nil", id)
}
acc := *accp
if len(acc) <= 0 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching accounting contact %d", id)
}
if len(u) <= 0 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching accounting contact by ID")
if len(acc) > 1 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("multiple results when fetching accounting contact %d", id)
}
accountingContact = u[0]
accountingContact = acc[0]

if accountingContact.ParentId == nil {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("accounting contact %d has no parent", id)
}

err = session.Read(odooclient.ResPartnerModel, []int64{accountingContact.ParentId.ID}, fetchPartnerFieldOpts, &u)
cpp, err := session.FindResPartners(
odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "=", id)),
fetchPartnerFieldOpts)
if err != nil {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching parent %d of accounting contact %d failed: %w", accountingContact.ParentId.ID, id, err)
}
if len(u) <= 0 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching parent %d of accounting contact %d failed", accountingContact.ParentId.ID, id)
if cpp == nil {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("fetching parent %d of accounting contact %d returned nil", accountingContact.ParentId.ID, id)
}
company = u[0]
cp := *cpp
if len(cp) <= 0 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("no results when fetching parent %d of accounting contact %d", accountingContact.ParentId.ID, id)
}
if len(cp) > 1 {
return odooclient.ResPartner{}, odooclient.ResPartner{}, fmt.Errorf("multiple results when fetching parent %d of accounting contact %d", accountingContact.ParentId.ID, id)
}
company = cp[0]

return company, accountingContact, nil
}
Expand All @@ -161,8 +179,7 @@ func (s *Odoo16Storage) List(ctx context.Context) ([]billingv1.BillingEntity, er
return nil, err
}

criteria := odooclient.NewCriteria().AddCriterion(roleAccountFilter).AddCriterion(activeFilter).AddCriterion(notInflightFilter)
accPartners, err := session.FindResPartners(criteria, fetchPartnerFieldOpts)
accPartners, err := session.FindResPartners(newValidInvoiceRecordCriteria(), fetchPartnerFieldOpts)
if err != nil {
return nil, err
}
Expand All @@ -176,7 +193,7 @@ func (s *Odoo16Storage) List(ctx context.Context) ([]billingv1.BillingEntity, er
companyIDs = append(companyIDs, int(p.ParentId.ID))
}

criteria = odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "in", companyIDs))
criteria := odooclient.NewCriteria().AddCriterion(activeFilter).AddCriterion(odooclient.NewCriterion("id", "in", companyIDs))
companies, err := session.FindResPartners(criteria, fetchPartnerFieldOpts)
if err != nil {
return nil, err
Expand Down Expand Up @@ -426,7 +443,6 @@ func mapBillingEntityToPartners(be billingv1.BillingEntity, countryIDs map[strin

func setStaticAccountingContactFields(conf Config, a *odooclient.ResPartner) {
a.CategoryId = odooclient.NewRelation()
a.CategoryId.AddRecord(int64(roleAccountCategory))
a.Lang = odooclient.NewSelection(conf.LanguagePreference)
a.Type = odooclient.NewSelection(invoiceType)
a.PropertyPaymentTermId = odooclient.NewMany2One(int64(conf.PaymentTermID), "")
Expand All @@ -449,3 +465,10 @@ func splitCommaSeparated(s string) []string {
}
return p
}

func newValidInvoiceRecordCriteria() *odooclient.Criteria {
return odooclient.NewCriteria().
AddCriterion(invoiceTypeFilter).
AddCriterion(activeFilter).
AddCriterion(notInflightFilter)
}
43 changes: 21 additions & 22 deletions apiserver/billing/odoostorage/odoo/odoo16/odoo16_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ func TestGet(t *testing.T) {
statusTime := st.Local()

gomock.InOrder(
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(456),
CreateDate: odooclient.NewTime(tn),
ParentId: odooclient.NewMany2One(123, ""),
Email: odooclient.NewString("[email protected], [email protected]"),
VshnControlApiMetaStatus: odooclient.NewString("{\"conditions\":[{\"type\":\"ConditionFoo\",\"status\":\"False\",\"lastTransitionTime\":\"" + statusTime.Format(time.RFC3339) + "\",\"reason\":\"Whatever\",\"message\":\"Hello World\"}]}"),
}}).Return(nil),
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
}}, nil),
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(123),
Name: odooclient.NewString("Test Company"),
}}).Return(nil),
}}, nil),
)

s, err := subject.Get(context.Background(), "be-456")
Expand Down Expand Up @@ -85,11 +85,10 @@ func TestGetNoParent(t *testing.T) {
defer ctrl.Finish()

gomock.InOrder(
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(456),
Name: odooclient.NewString("Accounting"),
}},
).Return(nil),
}}, nil),
)

_, err := subject.Get(context.Background(), "be-456")
Expand All @@ -101,12 +100,12 @@ func TestGet_ParentCantBeRetrieved(t *testing.T) {
defer ctrl.Finish()

gomock.InOrder(
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(456),
Name: odooclient.NewString("Accounting"),
ParentId: odooclient.NewMany2One(123, ""),
}}).Return(nil),
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("No such record")),
}}, nil),
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(nil, errors.New("No such record")),
)

_, err := subject.Get(context.Background(), "be-456")
Expand Down Expand Up @@ -189,17 +188,17 @@ func TestCreate(t *testing.T) {
// Reset inflight flag
mock.EXPECT().Update(odooclient.ResPartnerModel, gomock.InAnyOrder([]int64{700, 702}), gomock.Any()),
// Fetch created company
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(702),
Name: odooclient.NewString("Max Foobar"),
CreateDate: odooclient.NewTime(tn),
ParentId: odooclient.NewMany2One(700, ""),
Email: odooclient.NewString("[email protected], [email protected]"),
}}),
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
}}, nil),
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(700),
Name: odooclient.NewString("Test Company"),
}}),
}}, nil),
)

s := &billingv1.BillingEntity{
Expand Down Expand Up @@ -239,30 +238,30 @@ func TestUpdate(t *testing.T) {

gomock.InOrder(
// Fetch existing company
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(702),
ParentId: odooclient.NewMany2One(700, ""),
}}),
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
}}, nil),
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(700),
Name: odooclient.NewString("Test Company"),
}}),
}}, nil),
// Update company
mock.EXPECT().UpdateResPartner(gomock.Any()),
// Update accounting contact
mock.EXPECT().UpdateResPartner(gomock.Any()),
// Fetch created company
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(702),
CreateDate: odooclient.NewTime(tn),
ParentId: odooclient.NewMany2One(700, ""),
Email: odooclient.NewString("[email protected], [email protected]"),
VshnControlApiMetaStatus: odooclient.NewString("{\"conditions\":[{\"type\":\"ConditionFoo\",\"status\":\"False\",\"lastTransitionTime\":\"" + statusTime.Format(time.RFC3339) + "\",\"reason\":\"Whatever\",\"message\":\"Hello World\"}]}"),
}}),
mock.EXPECT().Read(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).SetArg(3, []odooclient.ResPartner{{
}}, nil),
mock.EXPECT().FindResPartners(gomock.Any(), gomock.Any()).Return(&odooclient.ResPartners{{
Id: odooclient.NewInt(700),
Name: odooclient.NewString("Test Company"),
}}),
}}, nil),
)

s := &billingv1.BillingEntity{
Expand Down
28 changes: 7 additions & 21 deletions ...doostorage/odoo/odoo16/odoo16mock/mock.go → ...ostorage/odoo/odoo16/odoo16mock/odoo16.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a8e87c3

Please sign in to comment.