Entities #58

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

@@ -324,7 +325,7 @@ func tokenListHelper(lr tokenListRetriever, opts *ListOptions) error {
 func addListOptions(requrl string, opts *ListOptions) (string, error) {
 	u, err := url.Parse(requrl)
 	if err != nil {
-		return "", nil
+		return "", err


and a bugfix - hurrah!

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

client.go

@@ -336,6 +337,7 @@ func addListOptions(requrl string, opts *ListOptions) (string, error) {
 		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

 func (h Hours) CustomFieldTag() string {
 	return CUSTOMFIELDTYPE_HOURS
 }

 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

@@ -531,6 +531,7 @@ func ParseCustomFields(cfraw map[string]interface{}, cfs []*CustomField) (map[st
 	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

@@ -42,11 +42,11 @@ func runParseTest(t *testing.T, index int, c customFieldParseTest) {
 	)

 	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

@@ -531,7 +531,6 @@ func ParseCustomFields(cfraw map[string]interface{}, cfs []*CustomField) (map[st
 	return parsed, nil
 }

-// 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

@@ -148,7 +150,7 @@ func isZeroValue(v reflect.Value, interpretNilAsZeroValue bool) bool {
 		return isZeroValue(v.Elem(), interpretNilAsZeroValue)
 	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

J=PC-40957 TEST=compile

* Add NewHoursClosedAllWeek static method

pr fixes

- newer API versions return custom fields differently. This change will give some backwards compatibility J=PC-219314 TEST=manual

#301) * migrate custom field endpoint to optionally use the configuration endpoint J=CR-3126 TEST=auto * refactor: respond to dev comments J=CR-3126

J=PC-220346

J=none TEST=manual Co-authored-by: sseverance <[email protected]>

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 <[email protected]>

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 <[email protected]>

This reverts commit d453c8d.

J=none

J=PC-258895

This reverts commit d840ea0.

J=none

… Get functions (#325)

Co-authored-by: Stephanie Severance <[email protected]>

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

tbirdsall and others added 24 commits January 28, 2019 11:37

Review Service: Fixed some review invitation issues (#76)

66b0933

J=PC-40957 TEST=compile

Add event entity support R=cdworak,usaha,bharvey (#69)

975df06

Updated entity code to accomodate varying Google Attributes fields (#77)

278d7ca

CustomFieldService -> LocationCustomFieldService; double pointer support

d928a6b

**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

nirmalpatel94 and others added 30 commits July 18, 2023 14:31

feat: add custom unmarshalling for newer V param's custom fields

34f39de

- newer API versions return custom fields differently. This change will give some backwards compatibility J=PC-219314 TEST=manual

migrate custom field endpoint to optionally use the configuration end… (

9bfb0c1

#301) * migrate custom field endpoint to optionally use the configuration endpoint J=CR-3126 TEST=auto * refactor: respond to dev comments J=CR-3126

feat: add config option to use EU host domain for yextapis.com (#302)

1d0529b

fix: update AnalyticsFilters FolderId name and type

ae2f67f

J=PC-220346

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

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

16a7daa

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

19383a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Entities #58

Entities #58

cdworak commented Jul 18, 2018

mhupman left a comment

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

cdworak Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

cdworak Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 4, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

mhupman Oct 19, 2018

cdworak Dec 10, 2018

Entities #58

Are you sure you want to change the base?

Entities #58

Conversation

cdworak commented Jul 18, 2018

mhupman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment