AP-982 | Attach images to notes/replies #558
Conversation
## Walkthrough
This change introduces support for optional image attachments in both note creation and reply actions within the application. The `GCPStorageService` is now injected into relevant API handlers to enable verification of image URLs. The request schemas for creating notes and replies are extended to accept an optional `imageUrl` field. When an image URL is provided, the service verifies its validity against the Google Cloud Storage bucket before proceeding. If verification fails, a 400 error response is returned. The `Post` entity and related methods are updated to store and handle the optional image URL. The publishing service and types are also modified to include image attachments in the ActivityPub note payloads when applicable. Comprehensive unit tests are added for both the new verification logic and the updated entity creation methods. Additionally, Cucumber feature tests and step definitions are extended to cover scenarios involving image URLs in notes and replies.
## Possibly related PRs
- TryGhost/ActivityPub#533: Introduces the initial integration of Google Cloud Storage, including a new image upload API and emulator setup; the current PR builds upon this by extending usage of the storage service for image URL verification.
- TryGhost/ActivityPub#472: Adds unit tests for `createNote` focusing on content formatting and error handling; related as it modifies the same method extended here to support image URLs.
## Suggested labels
`princi-vershwal-12.ghost.is`
## Suggested reviewers
- allouis📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/storage/gcloud-storage/gcp-storage.service.ts (1)
113-143: Good implementation, but consider enhancing emulator validationThe implementation of
verifyImageUrlproperly handles both emulator and production environments with good error handling. However, in emulator mode, it only checks if the URL matches the expected pattern but doesn't verify if the file actually exists.Consider enhancing the emulator mode to also verify file existence for consistency with production behavior:
async verifyImageUrl(url: string): Promise<boolean> { try { // Check if we're using the GCS emulator and verify the URL matches the emulator's base URL pattern if (this.emulatorHost) { const emulatorBaseUrl = `${this.emulatorHost.replace('fake-gcs', 'localhost')}/storage/v1/b/${this.bucketName}`; - return url.startsWith(emulatorBaseUrl); + if (!url.startsWith(emulatorBaseUrl)) { + return false; + } + + // Extract file path from emulator URL and verify existence + const filePath = url.split(`${emulatorBaseUrl}/o/`)[1]?.split('?')[0]; + if (!filePath) { + return false; + } + const decodedPath = decodeURIComponent(filePath); + const [exists] = await this.bucket.file(decodedPath).exists(); + return exists; } // Rest of the method remains unchanged
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/app.ts(2 hunks)src/handlers.ts(6 hunks)src/http/api/note.ts(3 hunks)src/post/post.entity.ts(4 hunks)src/post/post.entity.unit.test.ts(1 hunks)src/publishing/service.ts(2 hunks)src/publishing/types.ts(1 hunks)src/storage/gcloud-storage/gcp-storage.service.ts(1 hunks)src/storage/gcloud-storage/gcp-storage.service.unit.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app.ts (1)
src/http/api/note.ts (1)
handleCreateNote(17-70)
src/storage/gcloud-storage/gcp-storage.service.unit.test.ts (1)
src/storage/gcloud-storage/gcp-storage.service.ts (1)
GCPStorageService(14-144)
🪛 GitHub Actions: CICD
src/app.ts
[error] 959-1000: Biome formatting check failed: File content differs from formatting output. Please run the formatter to fix code style issues.
🔇 Additional comments (22)
src/publishing/types.ts (1)
106-109: LGTM!The addition of the
imageUrlproperty to the Note interface is well-documented and properly typed.src/post/post.entity.unit.test.ts (1)
312-322: Good test coverage for the new image URL featureThis test effectively verifies that the
createNotemethod correctly handles the newimageUrlparameter.src/publishing/service.ts (2)
8-8: LGTM!Importing the necessary
Imagetype for attachments.
146-152: Well-implemented conditional image attachmentThe implementation correctly handles attaching an image to the note when an image URL is provided, while maintaining backward compatibility by using
undefinedwhen no image is present.However, additional tests would be beneficial to ensure this functionality works correctly.
Could you verify that there are tests covering the image attachment functionality in the publishing service? Consider adding tests that verify the behavior when both with and without an image URL.
src/app.ts (2)
960-962: Good addition of storage service for image URL verificationThe injection of
gcpStorageServiceinto thecreateReplyActionHandlerenables verification of image URLs in replies. This matches the PR objective of attaching images to replies.
992-994: Good addition of storage service for note creationAdding
gcpStorageServicetohandleCreateNoteenables image URL verification for notes, fulfilling the PR objective of attaching images to notes.src/storage/gcloud-storage/gcp-storage.service.unit.test.ts (1)
125-214: Comprehensive test coverage forverifyImageUrlmethodThese tests thoroughly verify the
verifyImageUrlfunctionality in both emulator and production environments with appropriate test cases for:
- Valid URLs returning true
- Invalid URLs (wrong bucket, malformed) returning false
- Error handling returning false
- File existence checks in production mode
The separation into distinct test suites for emulator and production modes is well-structured and matches the implementation's conditional logic.
src/post/post.entity.ts (2)
327-331: Good implementation of optional image URL in createNoteThe method signature has been updated to accept an optional
imageUrlparameter and properly converts it to a URL object when provided. This change maintains backward compatibility while adding the new functionality.Also applies to: 355-355
369-374: Good implementation of optional image URL in createReplySimilar to
createNote, thecreateReplymethod has been updated to accept an optionalimageUrlparameter and properly converts it to a URL object. This implementation is consistent with thecreateNotemethod.Also applies to: 405-405
src/http/api/note.ts (6)
3-9: Good organization of importsImports are properly organized and typed, improving code readability and type safety.
14-14: Proper schema validation for imageUrlAdding Zod validation for the optional
imageUrlfield ensures that only valid URLs will be accepted. The use ofz.string().url()provides built-in URL validation.
17-22: Function signature updated with new dependencyAdding the
storageServiceparameter maintains the function's purpose while enabling image URL verification functionality.
31-40: Good implementation of image URL verificationThis code block:
- Checks if an image URL is provided
- Verifies the URL using the storage service
- Returns a meaningful error response if verification fails
This ensures that only valid image URLs from the configured storage bucket can be attached to notes.
44-44: Post creation updated to include image URLThe
Post.createNotecall now passes the optionalimageUrlparameter, connecting the API layer to the entity layer.
50-57: Publishing updated to include image URLThe
publishNotefunction call now includes the image URL in its payload, ensuring that the image attachment is included in the ActivityPub note object when published to the Fediverse.src/handlers.ts (7)
7-7: LGTM: Image import added to support image attachmentsThe Image class imported from @fedify/fedify will be used for attaching images to notes and replies.
22-22: LGTM: Required import for image verificationGCPStorageService type import is necessary for the storage service parameter added to the reply handler.
298-298: LGTM: Schema extension for image supportThe ReplyActionSchema is properly extended with an optional imageUrl field that includes URL validation.
305-305: LGTM: Added storage service parameterThe storageService parameter is correctly added to the handler for image URL verification.
321-330: LGTM: Image URL verification logicThe implementation properly verifies the image URL if provided and returns an appropriate error response if invalid.
437-442: LGTM: Updated Post creation with imageUrlThe Post.createReply call is correctly updated to include the imageUrl parameter.
451-457: LGTM: Image attachments in Note creationThe Note object correctly includes image attachments when an imageUrl is provided, using the Image class with the URL.
allouis
left a comment
allouis
left a comment
There was a problem hiding this comment.
Few comments on the types
src/post/post.entity.ts
Outdated
| static createNote( | ||
| account: Account, | ||
| noteContent: string, | ||
| imageUrl?: string, |
src/post/post.entity.ts
Outdated
| account: Account, | ||
| replyContent: string, | ||
| inReplyTo: Post, | ||
| imageUrl?: string, |
There was a problem hiding this comment.
Same here should be a URL!
| content: note.content, | ||
| summary: null, | ||
| published: Temporal.Now.instant(), | ||
| attachments: note.imageUrl |
There was a problem hiding this comment.
Did we test with image and both too? What were the results?
Ref https://linear.app/ghost/issue/AP-982 https://linear.app/ghost/issue/AP-1084
imageUrl. This is easier to work with for a single image-- Tested this will many options and setting it to the attachments using Image obj works. By works I mean image correctly shows up on mastodon.
-- Verifing the image url
-- Verifing the image url