Entities by cdworak · Pull Request #58 · yext/yext-go

cdworak · 2018-07-18T19:19:03Z

this isn't even really a pull request because it's not done, just so hup can see

mhupman

nice start! i agree the Register/Unwrap might need some tweaking, but only usage will tell for sure

mhupman · 2018-10-04T04:03:42Z

client.go

 	u, err := url.Parse(requrl)
 	if err != nil {
-		return "", nil
+		return "", err


and a bugfix - hurrah!

mhupman · 2018-10-04T04:04:24Z

client.go

 		q.Add("limit", strconv.Itoa(opts.Limit))
 	}
 	if opts.PageToken != "" {
+		// TODO: this is not compatible with the old parameter name


:(

maybe the func could take another parameter if the new/old apis are the same in everything other than param name

mhupman · 2018-10-04T04:05:07Z

customfield.go


 type DailyTimes struct {
-	DailyTimes string `json:"dailyTimes,omitempty"`
+	Sunday    string `json:"sunday,omitempty"`


was this wrong all along or a change in Entities?

a change in Entities...I copied over the old struct. same thing happened for hours. will figure out how to distinguish between the two

mhupman · 2018-10-04T04:06:11Z

customfield_service.go

 	return parsed, nil
 }

+// TODO: Can we do this anymore?


probably not, and it probably doesn't make sense. it was just to prevent the "obvious" mistake of passing in a string vs id. now we probably won't know until we send to the API (unless we validate against the field schema, but that is probably a new exposed method, not an automagic thing)

mhupman · 2018-10-04T04:06:36Z

customfield_service_test.go


 	if cfType != expectType {
-		t.Errorf("test #%d (%s) failed: expected type %s, got type %s", index, c.TypeName, expectType, cfType)
+		t.Errorf("test #%d (%s) failed:\nexpected type %s\ngot type %s", index, c.TypeName, expectType, cfType)


i really hate how hard it is to format things in test output

the tabbing in particular

mhupman · 2018-10-04T04:36:27Z

entity_service.go

+		return nil, fmt.Errorf("Unable to find entityType attribute in %v", metaByKey)
+	}
+
+	entityObj, err := e.LookupEntity(EntityType(entityType.(string)))


we should think about what we want to happen if a type is NOT registered. for example, what if the caller knew they didn't know the types ahead of time and wanted to just inspect them as JSON/map[string]interface{}? we might want to let that through and not error out in some cases?

on the other hand, i can also see it useful to know if you weren't able to unwrap the type, but maybe we find that our during type assertion?

mhupman · 2018-10-04T04:42:40Z

entity_service.go

+	// Convert into struct of Entity Type
+	entityJSON, err := json.Marshal(entityValsByKey)
+	if err != nil {
+		return nil, fmt.Errorf("Error marshaling entity to JSON: %s", err)


don't include "error" in error message or capitalize first word

mhupman · 2018-10-04T04:42:53Z

entity_service.go

+
+	err = json.Unmarshal(entityJSON, &entityObj)
+	if err != nil {
+		return nil, fmt.Errorf("Error unmarshaling entity JSON: %s", err)


mhupman · 2018-10-04T04:47:41Z

entity_service.go

+	return buf.Bytes(), nil
+}
+
+func (e *EntityService) toEntityTypes(entityInterfaces []interface{}) ([]Entity, error) {


this is throughout the PR, but the param names are stuttering with their type... prefer shortened values or descriptions of the content without the type:

toEntityTypes(entities []interface{})

yeah....i had a hard time because both the word type and interface you can't use in go, so i prefixed them a lot. I'll try to clean it up a bit

mhupman · 2018-10-04T04:48:09Z

entity_service.go

+	return entities, nil
+}
+
+func (e *EntityService) toEntityType(entityInterface interface{}) (Entity, error) {


similarly, toEntityType(entity interface{}) or toEntityType(e interface{})

mhupman · 2018-10-04T13:29:11Z

customfield_service.go

 }

-// TODO: Can we do this anymore?
 func validateCustomFieldsKeys(cfs map[string]interface{}) error {


maybe we could rename this to validateLocationCustomFields or something to indicate it is legacy

mhupman · 2018-10-04T13:30:16Z

registry.go

+
+func (r Registry) Register(key string, val interface{}) {
+	//From: https://github.com/reggo/reggo/blob/master/common/common.go#L169
+	isPtr := reflect.ValueOf(val).Kind() == reflect.Ptr


var (..) for all these

mhupman · 2018-10-04T13:37:41Z

entity.go

@@ -0,0 +1,22 @@
+package yext


yep, could embed to get Meta "for free" without need to duplicate. it would also be a good deser target if you didn't know what you were going to get from the API - for example, if you hit the generic entities endpoint to return any and all entities (assuming that exists?)?

mhupman · 2018-10-04T13:38:39Z

entity_test.go

+}
+
+func (l *CustomLocationEntity) Type() EntityType {
+	return ENTITYTYPE_LOCATION


Oh, you mean on create? If its pulled from the API, it would always be populated for you, right?

mhupman · 2018-10-04T13:38:52Z

error.go

+	return &Error{Type: typ, Code: codeInt, Message: message}, nil
+}
+
+func ErrorsFromString(errorStr string) ([]*Error, error) {


mhupman · 2018-10-04T13:39:00Z

event.go

@@ -0,0 +1,34 @@
+package yext


mhupman · 2018-10-19T19:57:24Z

cftasset_service.go

@@ -0,0 +1,148 @@
+package yext


why is CFT Asset a different service? can this be part of a unified AssetService?

mhupman · 2018-10-19T19:58:13Z

cftasset.go

@@ -0,0 +1,152 @@
+package yext


similarly, isn't this just asset.go?

mhupman · 2018-10-19T20:43:05Z

asset.go

@@ -1,20 +1,11 @@
 package yext


High level feedback discussed with @upalsaha:

Maintain AssetService but rename it to LegacyAssetService

Name the new service (with registry, entity and CFT support) AssetService

Add a switch in NewClient() that decides what service to create based on the Version date - leave the other service nil

cc @cdworak

mhupman · 2018-10-19T20:43:57Z

cftasset.go

+	ASSETTYPE_COMPLEXVIDEO AssetType = "complexVideo"
+)
+
+type CFTAsset struct {


per above, this would just be Asset - the old thing would become LegacyAsset

mhupman · 2018-10-19T20:44:38Z

cftasset.go

+}
+
+func (a *AssetUsage) Equal(b Comparable) bool {
+    defer func() {


why is this necessary?

mhupman · 2018-10-19T21:14:34Z

customfield.go

-	HolidayHours   []HolidayHours `json:"holidayHours,omitempty"`
+// HoursCustom is the Hours custom field format used by locations API
+// Entities API uses the Hours struct in location_entities.go (profile and custom hours are defined the same way for entities)
+type HoursCustom struct {


if we follow the other pattern, maybe LegacyHours?

mhupman · 2018-10-19T21:16:21Z

entity.go

+	b.nilIsEmpty = val
+}
+
+type RawEntity map[string]interface{}


another option is to do:

struct RawEntity {
Meta Meta
fields: map[string]interface
}

and implement json.Unmarshaler on it. then you wouldn't need to parse meta by hand

yeah i like this

mhupman · 2018-10-19T21:19:00Z

event.go

@@ -0,0 +1,34 @@
+package yext


bump on removal

mhupman · 2018-10-19T21:19:30Z

location_diff.go

 	case reflect.Struct:
 		for i, n := 0, v.NumField(); i < n; i++ {
-			if !isZeroValue(v.Field(i), interpretNilAsZeroValue) {
+			if !isZeroValue(v.Field(i), true) {


is this a change we want in location_diff

(vs entity_diff)

mhupman · 2018-10-19T21:20:22Z

location_entity.go

+
+func (y LocationEntity) SetLabelIds(v []string) {
+	l := UnorderedStrings(v)
+	y.SetLabelIdsWithUnorderedStrings(l)


can you inline the UnorderedStrings wrapper?

cdworak · 2018-12-10T19:06:00Z

location_entity.go

-	if y.LabelIds != nil {
-		v = *y.LabelIds
+func (y LocationEntity) GetLabels() (v UnorderedStrings) {
+	if y.BaseEntity.Meta.Labels != nil {


we should probably also move these functions under BaseEntity

* Add NewHoursClosedAllWeek static method

pr fixes

J=none TEST=manual Co-authored-by: sseverance <sseverance@yext.com>

J=PC-234251 TEST=auto

J=PC-220346

J=PC-237814 TEST=manual

- https://yexttest.atlassian.net/browse/PB-21550 J=PC-233325 TEST=manual

…310) J=PC-240725 TEST=manual

J=PC-240487 TEST=manual Co-authored-by: Lilian Wang <lwang@yext.com>

J=PC-244952

J=PC-244921 TEST=manual

J=none|TEST=manual

…endpoint to account for newer A (#315) J=PC-254056 TEST=manual

This reverts commit 919a4bf.

J=none|TEST=manual Co-authored-by: Peter Oliveira Soens <poliveirasoens@yext.com>

This reverts commit d453c8d.

J=none

J=PC-258895

This reverts commit d840ea0.

J=none

… Get functions (#325)

Co-authored-by: Stephanie Severance <sseverance@yext.com>

…s to set the type correctly (#329) J=PC-270239 TEST=manual

PC-279430

fix command

mhupman reviewed Oct 4, 2018

View reviewed changes

cdworak force-pushed the entities branch from 5ec35ee to 7356caa Compare October 4, 2018 14:45

mhupman reviewed Oct 19, 2018

View reviewed changes

upalsaha force-pushed the entities branch from eaba26a to 49d86e0 Compare October 26, 2018 13:34

cdworak commented Dec 10, 2018

View reviewed changes

cdworak and others added 24 commits February 8, 2019 17:11

**Date to **string

eecffe1

Add LocationType variables

d97d1ab

added cf endpoint location entitytype

02e0161

hours: Strip leading 0's

026f0fb

add external first party reviews create endpoint

3d51ed9

Merge branch 'createReviewsEndpoint' into entities

8f29470

Add lastLoginDate to Users struct PC-43487 (#84)

a58dc3f

update reviews service to handle response

976bb56

Merge branch 'reviewsUpdates' into entities

9f7df51

Add GetCountryCode() to LocationEntity

5916716

customfield_service helpers, add restuarant entity

2da3d76

Update variables for healthcare professional and healthcare facility

27a272a

Return Hours reference from SetClosedAllWeek (#89)

41976c6

* Add NewHoursClosedAllWeek static method

add healthcare_professional entity

350d8e8

unorderedstrings: if both nil, not diff

497c648

entity_diff: prevent diff from returning too early

b84ec9e

Add LocationType Event

318c71c

Update searchID param to searchIDs (#96)

d45e0aa

Create review diff, update pointers, and add getter to entities

9360a79

pr fixes

CustomFieldService: stop adding duplicates for multioption select ids

281c05b

modify healthcare professional entity

2a4c54c

Update hours helper struct serialize to return NullRegularHours()

23cf225

export helper functions (#103)

55f27ca

add entity ACL function to users_service (#100)

23f3b9c

junpark1213 and others added 30 commits January 5, 2024 12:16

fix: update click metric to new click count metric

ffd40cc

feat(hotel & restaurant): add additional fields to base struct (#304)

39bb0ba

J=none TEST=manual Co-authored-by: sseverance <sseverance@yext.com>

add product lists to financial professional entities (#306)

257a54d

J=PC-234251 TEST=auto

fix: remove deprecated metrics

399f2ae

J=PC-220346

fix(restaurant): add DineInHours to base struct

51f8fc3

J=PC-237814 TEST=manual

feat(reviews): add new review api fields

8d50bfc

- https://yexttest.atlassian.net/browse/PB-21550 J=PC-233325 TEST=manual

feat(custom): add nonbinary gender to healthcare professional entity (#…

f6d684a

…310) J=PC-240725 TEST=manual

feat(custom): change supported format struct for richtextv2 field (#311)

36f22ff

J=PC-240487 TEST=manual Co-authored-by: Lilian Wang <lwang@yext.com>

feat: add notifyUser field to user

70869ee

J=PC-244952

feat(entity_service): add convertRichTextToHTML param (#313)

7c11c04

J=PC-244921 TEST=manual

feat(hotel+restaurant): add addl fields (#314)

919a4bf

J=none|TEST=manual

fix(custom): allow for multiple data types of IDs from Review Update …

d47ec01

…endpoint to account for newer A (#315) J=PC-254056 TEST=manual

Revert "feat(hotel+restaurant): add addl fields (#314)" (#316)

2e0a927

This reverts commit 919a4bf.

feat(hotel+restaurant): add addl fields (#317)

d453c8d

J=none|TEST=manual Co-authored-by: Peter Oliveira Soens <poliveirasoens@yext.com>

Revert "feat(hotel+restaurant): add addl fields (#317)" (#318)

2c02c9d

This reverts commit d453c8d.

feat(assets): add folderids field

d5e98d6

J=none

feat(entities): add timestamp field to entity-meta

db1616c

J=PC-258895

feat(entities): add getter function for timestamp field

d818783

J=PC-258895

feat(entities): add GetTimestampAsTime function

c68fa76

J=PC-258895

Revert "feat(entities): add GetTimestampAsTime function"

bcdcdca

This reverts commit d840ea0.

feat(license): add license service (#323)

3696c38

J=none

feat(license): fix passing opts to tokenListHelper (#324)

c64ffde

J=none

feat(healthcare-professional): Add YextDisplayCoordinate property and…

75ab0d0

… Get functions (#325)

license-service: list license assignments in single object (#326)

c880e4a

Co-authored-by: Stephanie Severance <sseverance@yext.com>

entity: add CreatedTimestamp field to entityMeta struct (#327)

16a7daa

client: add optional Update-Operation-Id header to API requests (#328)

19383a4

fix(custom): add option for list CF types and proper fallback for CFT…

5256e02

…s to set the type correctly (#329) J=PC-270239 TEST=manual

fix(analytics): update ResponseRate type from int to float

c7481b3

PC-279430

chore: add instructions for making changes and deploying`

8e844f9

Update README.md

8a27bb5

fix command

Conversation

cdworak commented Jul 18, 2018

Uh oh!

mhupman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants