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

feat: use my.immich.app for externalDomain fallback #17209

Merged
merged 4 commits into from
Mar 31, 2025

Conversation

bo0tzz
Copy link
Member

@bo0tzz bo0tzz commented Mar 29, 2025

This is probably more useful than localhost.

This is probably more useful than localhost.
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Port is unused now

@@ -117,7 +117,7 @@ describe('/shared-links', () => {
const resp = await request(shareUrl).get(`/${linkWithAssets.key}`);
expect(resp.status).toBe(200);
expect(resp.header['content-type']).toContain('text/html');
expect(resp.text).toContain(`<meta property="og:image" content="http://`);
expect(resp.text).toContain(`<meta property="og:image" content="https://my.immich.app`);
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the right thing to do here?

Copy link
Member

Choose a reason for hiding this comment

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

It's weird but it's the correct test case, yes.

@zackpollard zackpollard merged commit e4f8368 into main Mar 31, 2025
45 checks passed
@zackpollard zackpollard deleted the feat/externaldomain-fallback branch March 31, 2025 11:08
kirill-dev-pro pushed a commit to kirill-dev-pro/immich-but-with-public-albums-within-instance that referenced this pull request Apr 6, 2025
* feat: use my.immich.app for externalDomain fallback

This is probably more useful than localhost.

* chore: remove port param

* fix: update expected value in tests

* fix: update expected value in e2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants