square / square-nodejs-sdk

Other
89 stars 40 forks source link

CatalogItem / ecomSeoData API should not return undefined. #151

Closed kyeshmz closed 4 months ago

kyeshmz commented 4 months ago

Describe the bug In the Squareup.com GUI, we can see that the SEO option populates avaliable fields using item name and description by default. This is not the case when calling it through the API. Also there is no option to make it the same as the item name, as this also renders the SEO field such as permalink to be undefined.

Expected behavior Permalink should return the default name, as is explained in the web GUI.

Currently, in the SDK ecomSeoData can be undefined, with the fields being able to be null. This should not be the case because they would be default defined as the name.

/** SEO data for for a seller's Square Online store. */
export interface CatalogEcomSeoData {
    /** The SEO title used for the Square Online store. */
    pageTitle?: string | null;
    /** The SEO description used for the Square Online store. */
    pageDescription?: string | null;
    /** The SEO permalink used for the Square Online store. */
    permalink?: string | null;
}

To Reproduce Steps to reproduce the bug:

  1. Create a menu with the necessary fields
  2. Try to retreieve the item and parse ecomSeoData.

Screenshots

Screenshot 2024-06-17 at 2 23 29 PM

Square SDK version "square": "^37.1.0",

zenmasterjobo commented 4 months ago

hi @kyeshmz

I agree this is a confusing interface, however this is currently working as expected.

Basically the ecomSeoData is using the item name and item description as the default values of those fields (pageTitle, pageDescription, permalink) but because they are just pointing to those fields which are set on the item itself, the seo data is not returned as part of the itemData.

If you change those fields in the GUI to something different, those values will return on the item when you query it through the API.

I raised this concern with the team though, and will update this ticket if they decide to change the behavior. Thank you for calling this out!

kyeshmz commented 4 months ago

I understand, like the idea, but not the implementation.

but because they are just pointing to those fields which are set on the item itself,

For example, if this was a pointer, this would not be null if the name or description was defined. It seems to me like it should just return both.