Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat(snippets): initial concept for snippet handling. #945

Merged
merged 9 commits into from
Aug 5, 2020

Conversation

elkmod
Copy link
Collaborator

@elkmod elkmod commented Jul 13, 2020

Changes

Provides snippets support for Shopware PWA

shopware-pwa snippets will only import snippets from Shopware and append them to your project locale file.

IMPORTANT You cannot create snippets from the admin and import them into the PWA.
First they have to be created within your locales/[iso-code].json file and exported to Shopware.

  1. Create PWA project with custom snippets
  2. Run shopware-pwa languages
  3. Run shopware-pwa snippets --export --username="admin" --password="shopware"
  4. Whenever changes to the snippet values were made, just run shopware-pwa snippets again to fetch those.
  5. Whenever you've added new snippet entries within your snippet files, push them to Shopware using --export

image
image

Checklist

  • I followed contributing guidelines

  • Error Handling

  • Hooking into the init process

  • Documentation

After Call with @patzick and @mkucmus

  • Merge strategy for synchronizing snippets (default: Shopware first, --pwa-first using --keep-local instead)
  • Only synchronize locales in locales directory

Proposed by @Rigo-m

  • Add global option to use custom namespace instead of pwa.

@vercel
Copy link

vercel bot commented Jul 13, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shopware-pwa/shopware-pwa-docs/ou19c057m
✅ Preview: https://shopware-pwa-docs-git-feat-snippet-handling-cli.shopware-pwa.vercel.app

@vercel vercel bot temporarily deployed to preview July 13, 2020 21:03 Inactive
@elkmod elkmod changed the title Don't merge - Initial concept for snippet handling. feat(snippets): initial concept for snippet handling. Jul 13, 2020
@elkmod elkmod requested review from patzick and mkucmus July 13, 2020 21:19
@Rigo-m
Copy link
Collaborator

Rigo-m commented Jul 14, 2020

Seems good! Maybe add an option to shopware-pwa.config.js to define the global namespace of the translations to change pwa?

@elkmod
Copy link
Collaborator Author

elkmod commented Jul 14, 2020

That's actually a good idea! That would allow for different snippet sets per project rather easily. Adding it to checklist.

@vercel vercel bot temporarily deployed to preview July 15, 2020 18:31 Inactive
@elkmod
Copy link
Collaborator Author

elkmod commented Jul 15, 2020

Added documentation for the command - it contains some matters to discuss. - https://shopware-pwa-docs-git-feat-snippet-handling-cli.shopware-pwa.vercel.app/landing/concepts/snippets.html#management-in-shopware

@Rigo-m
Copy link
Collaborator

Rigo-m commented Jul 15, 2020

Well done Dominic! It all makes sense to me. Do you have any doubts about some parts of the implementation?

@elkmod
Copy link
Collaborator Author

elkmod commented Jul 27, 2020

@Rigo-m, generally I'm fine with the concept, but it still lacks support for some complexities we could encounter, such as:

Multiple snippet sets for one language (you can configure different snippet sets for different shops, both based on the same locale). For example if you want to call it "Shopping Cart" within one Store (Sales Channel) and "My Basket" in the other one. We're currently using the locales to map the snippet sets and this will work in 99% of all the cases. However if we have the specific scenario outlined above, it fails to map the snippets correctly.

Hence, a proper solution would need to do the matching by snippet set ID instead of locale. This also means, that we need to refactor the shopware-pwa languages command, because it currently uses the Sales Channel languages to determine the languages present in PWA. However, the correct source of truth for language configurations are the Sales Channel domains (and inherently their snippet sets).

This will lead to a third point of issue which is the routing. We are using the language locale for routing within PWA, by prefixing every route with [locale]/ and then map it to the components within _lang. For the scenario mentioned above, the locale is ambiguous, because it can be used along with multiple snippet sets. The "quickest" way to work around that would be to use the snippet set ID instead of the locale for routing, which is obviously not an option, because no one wants routes like this:

my-pwa-store.com/4945f8fe6143465ea547fd627126353e/Clothing/Sweaters

So instead, we need to take the domains from the Sales Channel as a "base" of our routing together with the corresponding snippet sets. And based off those root domains we can do the routing per Sales Channel (language).

/cc @patzick .

@Rigo-m
Copy link
Collaborator

Rigo-m commented Jul 28, 2020

So how would the routing change based on the latest consideration?

I understand that shopware's internals are very well suited for multiple sales channel and multiple frontends, but since we are basically giving the developer the power to create snippets (since the end user will have to work with pwa's exported snippets and pwa is the source of truth) why not stick, for this case, for convention over configuration?
I.e.: Give the frontend developer the power to choose the snippet domain. By default it will be pwa and then if the frontend dev wants to export a different set of snippets for a different sales channel it can set it up with an option. Hence, this will not affect routing in any disruptive manner and will still provide enough flexibility to have multiple snippet sets for multiple sales channel pwa frontends.

@elkmod
Copy link
Collaborator Author

elkmod commented Jul 29, 2020

If we follow that path, we just have to make sure to be aware of that redundancy we introduce, because PWA won't reflect the domain configuration of your sales channel in Shopware but your language configuration instead. In that way it would functionally diverge from our default storefront implementation and at some point lead to confusion. For people relying on that very functionality.

I do appreciate your point of convention over configuration and using PWA as a "source of truth" for snippet management. This will allow us to decouple PWA from the sales channel configuration and also keep the locale codes as our primary identifier.

If we go down the path that you propose that will include less refactoring, but we'll keep this bit of architectural (debt/inconsistency) which we'll probably have to revise at a later stage.

We'll discuss those points tomorrow during our weekly and share the outcomes here. I think Patryk also has some thoughts on that which he'd like to share.

@vercel vercel bot temporarily deployed to preview July 30, 2020 07:32 Inactive
@vercel vercel bot temporarily deployed to preview August 5, 2020 12:58 Inactive
@vercel vercel bot temporarily deployed to preview August 5, 2020 13:06 Inactive
@github-actions
Copy link

github-actions bot commented Aug 5, 2020

💙 shopware-pwa-canary successfully deployed at https://a767417be8265893b74e1e3b621eb7a3876839bd.shopware-pwa-canary.preview.storefrontcloud.io

Copy link
Collaborator

@patzick patzick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

@patzick patzick merged commit 73e18dd into master Aug 5, 2020
@patzick patzick deleted the feat/snippet-handling-cli branch August 5, 2020 13:33
@patzick patzick added this to the v0.3.0 milestone Aug 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants