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

Add URL fields to shop settings #5144

Closed
aldeed opened this issue Apr 24, 2019 · 11 comments
Closed

Add URL fields to shop settings #5144

aldeed opened this issue Apr 24, 2019 · 11 comments

Comments

@aldeed
Copy link
Contributor

aldeed commented Apr 24, 2019

Background

This issue must be done before the following email issues can be resolved:

The common theme in these issues is that the API does not know what the storefront URLs are, so it can't use proper URLs when generating emails or redirecting after password resets, etc.

Work

Add the following fields to Shop schema:

  • storefrontHomeUrl
  • storefrontOrderUrl (can have :orderReferenceId and :orderToken in the string)
  • storefrontOrdersUrl (can have :accountId in the string)
  • storefrontAccountProfileUrl (can have :accountId in the string)

Alternatively they could be grouped in a storefrontUrls object on shop. They are all String type and optional with no default value.

Then add a new GraphQL mutation, updateShopUrls that allows admins to update these new fields.

Finally, add a Storefront URLs card on the shop settings UI page, with one text field for each of these.

The ability to set and change these is the outcome of this work. Further updates to use the values as necessary will be done separately after this is merged.

@aldeed
Copy link
Contributor Author

aldeed commented Apr 24, 2019

@rymorgan Any UI design comments? It's just 4 new text fields. I'm thinking on their own card? Ideally some help text or placeholder about what variables are recognized in each.

@rymorgan
Copy link
Contributor

@aldeed Yes please it's own card. These fields are overrides URLs that will be used by our transactional emails? Will they be used for anything else? Do they have to be filled in for the transactional emails work? I'm not clear on what they are used for by an operator. But my main request is that they are grouped logically and that their usage is spelled out for the user. I'm happy to help with that but I don't know enough yet. So clear logical title for the card. Clear labels on the forms that help a user know what these fields are for.

@kieckhafer
Copy link
Member

kieckhafer commented Apr 25, 2019

@rymorgan This is going to live on the Shops setting page. These fields are necessary for emails, however they might be useful for other reasons in the future, and they are settings on our Shops collection, so it seems to make sense here.

Unfortunately this uses some very old Blaze code to provide all these settings. I was just going to add a new accordion panel here. I'd really like to make it the new style, but I do not think at this time I have the time to refactor this whole screen into the new format:

Not-found

@kieckhafer
Copy link
Member

@rymorgan after our discussion, I moved this to the email card.

@machikoyasuda @aldeed will these fields ever be used anywhere outside of emails? Is this the best place for them?
Not-found

@aldeed
Copy link
Contributor Author

aldeed commented Apr 25, 2019

@kieckhafer @rymorgan They'll be used in other places, anywhere where we need to know the storefront URL. Examples:

  • To know where to redirect to after resetting your account password.
  • In SMS and in-app notifications that want to link to an order or to the storefront in general.
  • We might want to add a "Go to Storefront" link somewhere in the operator UI for convenience?
  • Possibly we should add one for productDetailUrl so that we could also have an "View on Storefront" button on the product editor?

So I would keep these on shop settings, not email settings page.

@rymorgan If it helps to make sense of this, the background is that historically storefront routes were in the same app so we knew what the URLs would be. But now storefronts are built custom by each installation and hosted elsewhere, so everything on the backend or in the operator UI has no idea what the routes for different pages look like. We need to tell it.

Although these fields are not required because they have no known default value, they are essentially required in that all of the things that are going to use it will throw errors if they aren't set. I think it would be great to have some sort of "action list" and/or "production readiness checklist" UI for operators. A place where you could click a button to run a check and it would report all issues with instructions for resolving them. In this case, the report would say "You haven't entered a storefront home page URL for your shop. Certain notifications and other features may require this to work properly. Click here to enter one." Just something to start thinking about design-wise.

@rymorgan
Copy link
Contributor

@aldeed @kieckhafer Based on @aldeed explanation I consider these fields required in term of UX. This should be part of onboarding into a store. The checklist idea is one I've thought about for years and haven't been able to get prioritized yet. It'd be part of an onboarding process and would likely live on an as of yet nonexistent dashboard. It would also have corresponding documentation about what is required in a getting started part of our docs.

Not sure how we address that here. Can we add a notification to the email settings that is persistent if these are not filled in? It will be very frustrating to have emails not working and not know why. This also has to go into our documentation.

@aldeed
Copy link
Contributor Author

aldeed commented Apr 25, 2019

Agree with all that. I think we lost in-app notifications entirely with the move to Catalyst, so we'd first have to move those back into the visible UI. I'm not sure how hard it would be to add a persistent notification after that. Otherwise I would say maybe just code in a persistent alert box at the top of the currently empty home page? Sort of a rough prototype of the "action items" list?

@kieckhafer
Copy link
Member

I've created #5150 to continue discussion of the UI portion of this ticket, as it seems the GraphQL portion needs to be merged ASAP to help out with the above mentioned tickets, and that's done here.

@rymorgan
Copy link
Contributor

Wait, we don't have notifications in Catalyst UI?! Uh oh.

I don't mind that idea for a first pass of a checklist and as a reason for the "home" dashboard view to exist.

@spencern
Copy link
Contributor

I think that may be correct, but this is the first time I've thought of that.
Our in-app notification system was fairly brittle before, so I don't know if we would have been able to add something like this easily, but still a regression in terms of functionality that was unaccounted for.

@aldeed
Copy link
Contributor Author

aldeed commented Apr 29, 2019

Done

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

No branches or pull requests

4 participants