Conversation
| } catch (e) { | ||
| return send502(e); | ||
| } | ||
| for (const header of [ 'content-type', 'etag', 'last-modified', 'content-disposition', 'cache-control' ]) { |
There was a problem hiding this comment.
This is the set of useful headers I can proxy through without causing any problems if the original is gzip encoded.
There was a problem hiding this comment.
I think eslint --fix did this.
| for (const file of files) { | ||
| if (self.options.prettyUrls) { | ||
| const { extension } = file.attachment; | ||
| file._url = `${req.baseUrl}${self.options.prettyUrlDir}/${file.slug.replace(self.options.slugPrefix || '', '')}.${extension}`; |
There was a problem hiding this comment.
The slugPrefix isn't "pretty" (/files/file-name is not as nice as /files/name)
Slug without prefix is still unique among pieces of this type.
There was a problem hiding this comment.
This will blow up static builds.
const baseUrl = self.apos.url.getBaseUrl(req, { prefix: true })The Base URL should respect:
- prefix
- static base URL if set
- etc - all rules applied by the single source of truth now above
if prefix shouldn't be respected, prefix: false can be passed.
Even with that, static build should be also validated, because
- we expect relative URLs when getting metadata
- and single base URL for the attachments
- I doubt this will be included in the attachments metadata at all
- We ignore the files and images modules metadata when processing "used" attachments, but
filescan register these URLs as "literal content" I guess, which can save the day - If we do that, static build will have duplicated (different paths) files, but this is expected and the right thing to do
| if (self.options.prettyUrls) { | ||
| const { extension } = file.attachment; | ||
| file._url = `${req.baseUrl}${self.options.prettyUrlDir}/${file.slug.replace(self.options.slugPrefix || '', '')}.${extension}`; | ||
| file.attachment._prettyUrl = file._url; |
There was a problem hiding this comment.
A hint to .url() for later
| } | ||
| return { | ||
| get: { | ||
| async [`${self.options.prettyUrlDir}/*`](req, res) { |
There was a problem hiding this comment.
Implements /files/pretty-name-here.pdf
BoDonkey
left a comment
BoDonkey
left a comment
There was a problem hiding this comment.
I;m not sure if my comment will show up or not. I was commenting on Miro's point and asking if the helper utility should be used instead of req.baseUrl
|
I made the change. |
3 participants
A simple and effective implementation. Optional because there is a performance tradeoff but this is a reasonably performant solution. I don't think we should implement it for images, because it would hurt performance more significantly, and RA agrees that's not important. People care much more for PDFs because they are pages, of a sort, and people see that URL.