cfpb / wagtail-sharing

Easier sharing of Wagtail drafts
Creative Commons Zero v1.0 Universal
55 stars 14 forks source link

Share URL functionality includes port when deployed. #77

Open alexboots opened 2 weeks ago

alexboots commented 2 weeks ago

Current behavior

The get_sharing_url function generates an URL that has the port in it. That is fine for localhost but once deployed that breaks the share URL functionality. There is no easy way to hook into this menu via wagtail hooks to change this.

Expected behavior

There should be a way to remove the port from the Sharing Url, so it shows up like this: example.com/some-url/

It currently shows up at example.com:8081/some-url/

Steps to replicate behavior (include URLs)

  1. Setup wagtail-sharing
  2. Deploy to URL (in this case Google Cloud, App Engine standard environment)
  3. Set port as 8081 to get it working (for that env)
  4. See the link to the Staging URL generated has the port in it which makes the link break.

I don't see an easy fix for this outside of making a PR to add the option to enable / disable port in the sharing URL depending on the env.

Screenshots

Screenshot 2024-11-07 at 6 14 09 PM
chosak commented 2 weeks ago

Hi @alexboots. It seems like you want the regular Wagtail server and the preview URLs to have different ports. Can you modify the sharing site port via the UI to make this work? For example, at http://localhost:8000/admin/snippets/wagtailsharing/sharingsite/ locally?

alexboots commented 2 weeks ago

Hey chosak!

The sites are served on the same port, when I have either 443 or 80 set as the port in both of these locations: /admin/sites/ and /admin/snippets/wagtailsharing/sharingsite/

The sharing URL does not have the port in it, but the sharing site functionality does not work (the page 404's).

When I set them to the port Google App Engine's standard environment runs on, which is 8081, the port shows up in the URL (exampel.com:port/post-slug) which causes the sharing link to not work. I'm guessing due to this code here.

I am not sure why google's App Engine listens on 8081 instead of 443, but its set internally and we don't have access to change it. When I have both et to 80801, and I visit the URL without the port number in it, the sharing page does work as expected! But the sharing URL isn't correct.

chosak commented 2 weeks ago

@alexboots thanks for the additional detail. If you set the sharing site port to 80, the sharing URL should be http://whatever without a port, and if you set it to 443 it should use https:// without a port (per the code link you shared). Do either of these get you what you need?

The wagtail-sharing SharingSite model should be totally independent from whatever you have configured in the Wagtail Site model. I'm not super familiar with GAE configuration, so perhaps there's some piece I'm missing here.

alexboots commented 2 weeks ago

The Share page only seems to work on GAE standard environment if the share port is set to 8081 in /admin/snippets/wagtailsharing/sharingsite/

I set it to 80 => refresh example.com/url-for-draft => 404 I set it to 443 => refresh example.com/url-for-draft => 404 I set it to 8080 => refresh example.com/url-for-draft => 404 I set it to 8081 => refresh example.com/url-for-draft => Works as expected and shows 'This is Sharing site' banner

I think GAE Standard environment is setup to use 8081 instead of the typical 443 port? I tried 8081 because I saw this in the servers logs: [pid1-nginx] Successfully connected to localhost:8081 after 361.638431ms and it worked. I don't fully grok how wagtail-sharing is using port under the hood to get whats going on here 🤔 .

edit: Maybe a decent fix would be to make the port passed to here optionally configurable

chosak commented 1 week ago

@alexboots do you have one single GAE domain on which you are trying to use 2 separate ports? Or do you have 2 different domains pointing to the same GAE app?

wagtail-sharing can only see whatever host and port come in with the request object. See the Django docs on this here.

It sounds like you have this working with 8081 for the sharing URL?

alexboots commented 1 week ago

@chosak This is probably important context I left out! I am using the default domain that comes with GAE - it is an internal CMS so we are not configuring a specific public domain for it.

When using GAE you can push up 'versions' of the app that get their own URL. We have the same code pushed to each version, and they are both referencing the same database.

So we have the primary version where admins edit things: https://.uc.r.appspot.com/

And the preview version which the share link leverages: https://preview-dot-.uc.r.appspot.com/

I couldn't get the preview-dot- version to show the sharing page (the banner was not appearing) and saw in the Cloud Explorer Logs port 8081 being mentioned so tried it and it started working. And that port is not configurable on GAE standard environment side of things, and it seems to attach to 8081 which makes the port show up in the share URL.

I wrote some JS to remove the port for now for this scenario:

document.addEventListener("DOMContentLoaded", (event) => {
  if(window.location.host === '<app-id>.uc.r.appspot.com' ||
    window.location.host === 'localhost:8000'
  ) {
    let actionsButton = document.querySelector("header nav[aria-label='Actions']");
    // Header menu button
    if(actionsButton) {
      changeShareUrl(actionsButton);
    }

    // List posts page
    let actionsButtonList = document.querySelectorAll("#listing-results ul.actions");
    if(actionsButtonList) {
      for (let button of actionsButtonList) {
        changeShareUrl(button);
      }
    }
  }
});

const changeShareUrl = (actionsButton) => {
  actionsButton.addEventListener("click", (event) => {
    setTimeout(() => {
      let tippyContent = document.querySelector('[data-tippy-root]');
      let links = tippyContent.getElementsByTagName('a');
      for (let link of links) {
        if(link.innerText === 'View sharing link') {
          // This line doesn't event localhost
          link.href = link.href.replace(':8081', '');
          link.setAttribute('target', '_blank');
        }
      }
    }, 100);
  });
}