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

feature: proxy files with custom slugs #224

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jbmoelker
Copy link
Member

@jbmoelker jbmoelker commented Dec 7, 2024

Changes

Makes files available under custom URLs by

  • Adding an optional custom slug field to the File model.
  • Adding middleware to proxy serve files under their custom slugs. For example /my-files/example.pdf serves https://www.datocms-assets.com/145765/1730925112-dummy.pdf.

Associated issue

Resolves #155

How to test

  1. Open preview link
  2. Navigate to the File Demo
  3. Try the part under "File with a custom slug"
  4. Verify that the file is proxied (available) under a custom slug
  5. Play around with custom file slugs in the CMS
  6. Verify your changes work (will need to run branch locally as deploy preview doesn't update)

Checklist

  • I have performed a self-review of my own code
  • I have made sure that my PR is easy to review (not too big, includes comments)
  • I have made updated relevant documentation files (in project README, docs/, etc)
  • I have added a decision log entry if the change affects the architecture or changes a significant technology
  • I have notified a reviewer

@jbmoelker jbmoelker changed the title feat: proxy files with custom slugs (WIP) feature: proxy files with custom slugs Dec 17, 2024
Copy link

cloudflare-workers-and-pages bot commented Dec 23, 2024

Deploying head-start with  Cloudflare Pages  Cloudflare Pages

Latest commit: d40ec53
Status: ✅  Deploy successful!
Preview URL: https://0df38499.head-start.pages.dev
Branch Preview URL: https://feat-proxy-files-with-custom.head-start.pages.dev

View logs

@@ -10,5 +10,6 @@ fragment FileRoute on FileRecord {
url
}
locale
customSlug: slug
Copy link
Member Author

Choose a reason for hiding this comment

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

required to resolve clashing types with other slug fields on route fragments

Copy link
Contributor

Choose a reason for hiding this comment

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

This would also be resolved when renaming the field

@@ -0,0 +1,37 @@
import { defineMiddleware } from 'astro/middleware';
Copy link
Member Author

@jbmoelker jbmoelker Dec 23, 2024

Choose a reason for hiding this comment

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

Using the new atomic middleware handlers setup introduced in #222 . Should maybe be merged first?

@jbmoelker jbmoelker marked this pull request as ready for review December 28, 2024 19:59
id: 'cnnDYybJTAGvHxiYq2MJ8g',
label: 'Slug',
field_type: 'string',
api_key: 'slug',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think naming this slug is confusing. In my perception, a slug is a human-readable, yet machine-safe path fragment. What is described in the hint, and in the customFields function seems to show a path, not a slug

environment: datocmsEnvironment,
});

const fileSlugMap = {} as FileSlugMap;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const fileSlugMap = {} as FileSlugMap;
const fileSlugMap: FileSlugMap = {};

This is safer than as:

image

@@ -10,5 +10,6 @@ fragment FileRoute on FileRecord {
url
}
locale
customSlug: slug
Copy link
Contributor

Choose a reason for hiding this comment

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

This would also be resolved when renaming the field

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.

Specific file URLs
2 participants