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

More resilient image names #82

Closed
hatton opened this issue Sep 26, 2023 · 10 comments
Closed

More resilient image names #82

hatton opened this issue Sep 26, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@hatton
Copy link
Member

hatton commented Sep 26, 2023

Currently, we extract an image name from the S3 URL that Notion uses. This name "matters" if you're doing image localization.

My concern is, what if Notion changes their URL scheme in a way that either a) requires a new regex to extract it (this has happened already once) or b) changes the id entirely?

Instead, we should use the id of the block, which is likely to be at least as stable, if not more-so, and doesn't require a regext to extract. here's a typical block with unnecessary stuff removed:

{
    "object": "block",
    "id": "690583ea-a11a-479a-b66d-b566eb1a52aa",   <--- we should use this for the image name

    "type": "image",
    "image": {
      "type": "file",
      "file": {
        "url": "https://s3.us-west-2.amazonaws.com/secure.notion-static.com/7c53cd0c-f6e6-43be-b1d0-e39124615294/Untitled.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Credential=AKIAT73L2G45EIPT3X45%2F20230926%2Fus-west-2%2Fs3%2Faws4_request&X-Amz-Date=20230926T154205Z&X-Amz-Expires=3600&X-Amz-Signature=82c0a4710cd112e0b3ff1b1fc60562747ad9b541891141ee3be0f97ac12f67bf&X-Amz-SignedHeaders=host&x-id=GetObject",
        "expiry_time": "2023-09-26T16:42:05.665Z"
      }
    }
  },

This is an easy fix, however it would break existing users, so we need a way to opt in or opt out of this change.

@hatton hatton added the enhancement New feature or request label Sep 26, 2023
@Nateowami
Copy link
Contributor

This is currently a major issue for the Scripture Forge help site. Unfortunately I don't have time now to address this, but I wanted to register the fact that we will benefit once this is addressed.

@andrew-polk
Copy link
Contributor

@Nateowami
Bloom and Paratext are both using the system as is with two different strategies for localization.
If it feels like this is holding you back, it might be worth a short conversation to understand the hang-up and how these two sites are working with the tension currently.

@Nateowami
Copy link
Contributor

@andrew-polk Originally I planned to have our site be built by running a script that would fetch all documents from Notion, Crowdin, and elsewhere, and build the site. None of the content would be checked in (no screenshots either).

However, I realized before too long that this was kind of impractical and would make it difficult to understand what had actually changed. So I created a new repo, and content (docs and images) are checked in.

Having the file names change every time we fetch them means new screenshots are created in the repo every time. Having the markdown change also added some challenges with updating Crowdin, since there were always changes to the files.

I can't think of a good solution that doesn't involve keeping the file names consistent.

@andrew-polk
Copy link
Contributor

Are the file names changing every time you run docu-notion? Or just when you change something with the image?

With our and PTX setup, we only get changes if there are changes in Notion.

If you see a change with every pull, we will need to figure out what is causing that.

@Nateowami
Copy link
Contributor

Yes, they change every time, even a few minutes later. I'm running it with npx docu-notion -n $SF_HELP_NOTION_TOKEN -r $SF_HELP_NOTION_ROOT_PAGE_ID

@andrew-polk
Copy link
Contributor

Hm. Looks like npx is running docu-notion 0.11.0.
I'll look into why, but in the meantime, try using 0.15.0.

@Nateowami
Copy link
Contributor

I thought npx downloads the latest version of docu-notion and runs it. Does npx not work how I think? How do I tell it to use a particular version?

@Nateowami
Copy link
Contributor

Oh, it looks like I should be using @sillsdev/docu-notion instead of docu-notion

@andrew-polk
Copy link
Contributor

Ah. That would do it.
Also, apparently npx @sillsdev/docu-notion@latest guarantees no cache issues, etc.

andrew-polk added a commit that referenced this issue Apr 5, 2024
------------------
Breaking change:
Previously, image files names were a hash of all or part of the image url.
To provide more stability and future-proofing, the default format is now `{page-slug}.{notion-block-id}`. (#82)
Users can opt in to the old format with `--image-file-name-format legacy`.

------------------

Feature:
If desired instead, users can specify `--image-file-name-format content-hash` to use a hash of the image content as the file name. (#76)

------------------

Potential breaking change for plugins:
The exported type IDocuNotionContext changed from 
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown;
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
to
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown; 
  pageInfo: IDocuNotionContextPageInfo; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
where `IDocuNotionContextPageInfo` is
```
export type IDocuNotionContextPageInfo = { 
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  slug: string; 
}; 
```
andrew-polk added a commit that referenced this issue Apr 8, 2024
------------------
Breaking change:
Previously, image files names were a hash of all or part of the image url.
To provide more stability and future-proofing, the default format is now `{page-slug}.{notion-block-id}`. (#82)
Users can opt in to the old format with `--image-file-name-format legacy`.

------------------

Feature:
If desired instead, users can specify `--image-file-name-format content-hash` to use a hash of the image content as the file name. (#76)

------------------

Potential breaking change for plugins:
The exported type IDocuNotionContext changed from 
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown;
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
to
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown; 
  pageInfo: IDocuNotionContextPageInfo; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
where `IDocuNotionContextPageInfo` is
```
export type IDocuNotionContextPageInfo = { 
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  slug: string; 
}; 
```
andrew-polk added a commit that referenced this issue Apr 8, 2024
------------------
Breaking change:
Previously, image files names were a hash of all or part of the image url.
To provide more stability and future-proofing, the default format is now `{page-slug}.{notion-block-id}`. (#82)
Users can opt in to the old format with `--image-file-name-format legacy`.

------------------

Feature:
If desired instead, users can specify `--image-file-name-format content-hash` to use a hash of the image content as the file name. (#76)

------------------

Potential breaking change for plugins:
The exported type IDocuNotionContext changed from 
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown;
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
to
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown; 
  pageInfo: IDocuNotionContextPageInfo; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
where `IDocuNotionContextPageInfo` is
```
export type IDocuNotionContextPageInfo = { 
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  slug: string; 
}; 
```
andrew-polk added a commit that referenced this issue Apr 8, 2024
------------------
Breaking change:
Previously, image files names were a hash of all or part of the image url.
To provide more stability and future-proofing, the default format is now `{page-slug}.{notion-block-id}`. (#82)
Users can opt in to the old format with `--image-file-name-format legacy`.

------------------

Feature:
If desired instead, users can specify `--image-file-name-format content-hash` to use a hash of the image content as the file name. (#76)

------------------

Potential breaking change for plugins:
The exported type IDocuNotionContext changed from 
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown;
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
to
```
export type IDocuNotionContext = { 
  layoutStrategy: LayoutStrategy; 
  options: DocuNotionOptions; 
  getBlockChildren: IGetBlockChildrenFn; 
  notionToMarkdown: NotionToMarkdown; 
  pageInfo: IDocuNotionContextPageInfo; 
  convertNotionLinkToLocalDocusaurusLink: (url: string) => string | undefined; 
  pages: NotionPage[]; 
  counts: ICounts; 
  imports: string[]; 
}; 
```
where `IDocuNotionContextPageInfo` is
```
export type IDocuNotionContextPageInfo = { 
  directoryContainingMarkdown: string; 
  relativeFilePathToFolderContainingPage: string; 
  slug: string; 
}; 
```
@andrew-polk
Copy link
Contributor

Default image file name template is now {page-slug}.{notion-block-id}.
See https://github.com/sillsdev/docu-notion/releases/tag/v0.16.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants