magento / pwa-studio

šŸ› Development tools to build, optimize and deploy Progressive Web Applications for Magento 2.
https://developer.adobe.com/commerce/pwa-studio/
Open Software License 3.0
1.06k stars 683 forks source link

Venia does not use store configuration base URLs #567

Closed ericerway closed 5 years ago

ericerway commented 5 years ago

Configuration of media directories can vary between installations of Magento, but the Venia Concept storefront is strongly opinionated that you're using /pub as your document root. We should instead infer media urls from the below GraphQL query, and update venia-upward.yml and util/makeUrl.js to be more flexible when detecting and proxying media requests.

query {
  storeConfig {
    secure_base_media_url
  }
}

This issue is for the following packages:

This issue is a:

Expected result:

Only changing the MAGENTO_BACKEND_URL environment variable, Venia should support serving media from Magento instances using /pub or / as their document root. This can also be customized via the admin, so we should also support custom values, ie. {{secure_base_url}}my/media/path/ entered in Secure Base URL for User Media Files.

milanstojanov90 commented 5 years ago

Hello, as a reporter of this bug on Twitter, I'm glad to provide more details on this.

Environment

OS: Mac OS X Mojave NodeJS version: v8.12.0 npm version: 6.4.1

Installation

In my local web root /usr/local/var/www/magento23 I have 2 folders:

In my /usr/local/var/www/magento23/pwa-studio/packages/venia-concept/.env file I have updated only this: MAGENTO_BACKEND_URL="http://dev.pwastudio.com/"

Run the server

screen shot 2018-11-29 at 10 30 43 pm

CMS Page (note the port is 10000, the same is if I change URL and type 8049)

screen shot 2018-11-29 at 10 33 36 pm

Category Page

screen shot 2018-11-29 at 10 35 27 pm

Product Page

screen shot 2018-11-29 at 10 36 57 pm

Checkout

Note how the error is not displayed on the frontend. It's shown in the developer tools though. Whatever I type here, I can't see the "Save" button enabled. It always stays disabled.

screen shot 2018-11-29 at 10 38 53 pm
deepanshu27193 commented 5 years ago

Same Here.

Only thing I changed is: MAGENTO_BACKEND_URL="http://127.0.0.1/magento23pwa/"

ericerway commented 5 years ago

Thanks for all the great details. I'll defer the troubleshooting to @zetlen -- but had one question, does this happen with the example M2.3.0 cloud instance in .env.dist as well or just the updated MAGENTO_BACKEND_URL?

milanstojanov90 commented 5 years ago

Yes, it does! Because it took me a long time to figure out the installation process, at some point I tried with the default MAGENTO_BACKEND_URL, but the results were the same.

ericerway commented 5 years ago

Got it. Thanks for clarifying, we'll take a look. šŸ‘

deepanshu27193 commented 5 years ago

At my end, It's working perfect with the default cloud URL. Only changing the MAGENTO_BACKEND_URL to local codebase gives the problem.

I have found the code which is expecting Magento to be hosted from the pub directory rather than from the root directory. Issue is at: https://github.com/magento-research/pwa-studio/blob/release/2.0/packages/venia-concept/src/util/makeMediaPath.js#L19

Solution: Create a virtual host and point it to the pub directory instead of the root directory.

Now no more 404 on category and product images. Everything seems to be working now.

tjwiebell commented 5 years ago

@zetlen

There are two problems here that I think can be easily resolved.

1) venia-upward.yml assumes requests for media/ should just proxy to env.MAGENTO_BACKEND_URL, which breaks if your instance is not using /pub as your document root. As @deepanshu27193 pointed out, updating M2 server config resolved their problem. I think we can just document how to configure Venia to match your M2 configuration; split out that response rule and proxy to a mediaProxy target instead (env.MAGENTO_BACKEND_URL + /pub).

2) Validation for state field in shipping form doesn't bubble up to the user. Depending if we have UX mockups for validation, I think we just need to better indicate form validation errors, or better yet; just change the input type.

If there's a preferred path to resolution here, please update the ticket, and I'm available to resolve if you want to re-assign.

tonai commented 5 years ago

I got the same problem when trying to use my local instance of Magento.

I updated the venia-upward.yml and updated the pattern for the proxy with ^/(graphql|rest|media|pub/).

And I added in my .env file:

MAGENTO_BACKEND_MEDIA_PATH_PRODUCT="/pub/media/catalog/product"
MAGENTO_BACKEND_MEDIA_PATH_CATALOG="/pub/media/catalog/category"
Nastradoomus commented 5 years ago

So this is the problem:

.env file default: MAGENTO_BACKEND_PRODUCT_MEDIA_PATH="/media/catalog/product" MAGENTO_BACKEND_CATEGORY_MEDIA_PATH="/media/catalog/category"

Should be: MAGENTO_BACKEND_MEDIA_PATH_PRODUCT="/pub/media/catalog/product" MAGENTO_BACKEND_MEDIA_PATH_CATALOG="/pub/media/catalog/category"

zetlen commented 5 years ago

Long overdue, but:

Suggestion: Let's rename this issue "Venia does not use store configuration base URLs" and welcome efforts from our community members to integrate a storeConfig query into the UPWARD server-side render.