instagram stories, add collaborators, infrastructure for settings in validation #468
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes include modifications to several components and files related to post management within a frontend application and a NestJS backend. Key updates involve enhancing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }, | ||
| ]; | ||
| const InstagramCollaborators: FC<{ values?: any }> = (props) => { | ||
| const { watch, register, formState, control } = useSettings(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
| }, | ||
| ]; | ||
| const InstagramCollaborators: FC<{ values?: any }> = (props) => { | ||
| const { watch, register, formState, control } = useSettings(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
| setTagValue(modify); | ||
| onChange({ target: { value: modify, name } }); | ||
| }, | ||
| [tagValue] |
Check warning
Code scanning / ESLint
verifies the list of dependencies for Hooks like useEffect and similar Warning
| setTagValue(modify); | ||
| onChange({ target: { value: modify, name } }); | ||
| }, | ||
| [tagValue] |
Check warning
Code scanning / ESLint
verifies the list of dependencies for Hooks like useEffect and similar Warning
| if (settings) { | ||
| setTagValue(settings); | ||
| } | ||
| }, []); |
Check warning
Code scanning / ESLint
verifies the list of dependencies for Hooks like useEffect and similar Warning
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (4)
apps/frontend/src/components/launches/providers/instagram/instagram.collaborators.tsx (1)
19-19: Remove unused variables 'formState' and 'control'Variables
formStateandcontrolare assigned but never used. Removing them can improve code readability and eliminate unnecessary code.Apply this diff to remove the unused variables:
- const { watch, register, formState, control } = useSettings(); + const { watch, register } = useSettings();🧰 Tools
🪛 GitHub Check: ESLint
[warning] 19-19: Disallow unused variables
'formState' is assigned a value but never used.
[warning] 19-19: Disallow unused variables
'control' is assigned a value but never used.libraries/nestjs-libraries/src/integrations/social/instagram.provider.ts (2)
210-210: Remove debug console.log statements.Debug console.log statements should not be committed to production code. Consider using a proper logging system if logging is needed.
- console.log('in progress'); - console.log('in progress1'); - console.log(collaborators); - console.log('in progress2'); - console.log('in progress3');Also applies to: 232-232, 241-241, 250-250, 262-262
234-239: Consider extracting collaborators logic to a separate method.The collaborators logic could be extracted to improve readability and reusability.
+ private getCollaboratorsParam(settings?: InstagramDto, isStory: boolean): string { + if (!settings?.collaborators?.length || isStory) { + return ''; + } + return `&collaborators=${JSON.stringify( + settings.collaborators.map((p) => p.label) + )}`; + }Then use it in the code:
- const collaborators = - firstPost?.settings?.collaborators?.length && !isStory - ? `&collaborators=${JSON.stringify( - firstPost?.settings?.collaborators.map((p) => p.label) - )}` - : ``; + const collaborators = this.getCollaboratorsParam(firstPost?.settings, isStory);apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
71-80: LGTM! Consider adding JSDoc documentation.The addition of generic type parameter
Tandsettingsparameter improves type safety. However, documentation would help explain the expected shape and usage of these parameters.Add JSDoc documentation above the function:
+/** + * Higher-order component that provides social media post management functionality + * @template T - Type of settings object used for validation + * @param SettingsComponent - Optional component for rendering settings UI + * @param CustomPreviewComponent - Optional component for custom preview + * @param dto - Optional DTO for form validation + * @param checkValidity - Optional callback for validating posts with settings + * @param maximumCharacters - Optional maximum character limit + */ export const withProvider = function <T extends object>(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
apps/frontend/src/components/launches/add.edit.model.tsx(1 hunks)apps/frontend/src/components/launches/helpers/use.values.ts(3 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(1 hunks)apps/frontend/src/components/launches/providers/instagram/instagram.collaborators.tsx(1 hunks)apps/frontend/src/components/launches/providers/instagram/instagram.provider.tsx(0 hunks)apps/frontend/src/components/launches/providers/instagram/instagram.tags.tsx(1 hunks)apps/frontend/src/components/launches/providers/show.all.providers.tsx(1 hunks)libraries/nestjs-libraries/src/dtos/posts/providers-settings/instagram.dto.ts(1 hunks)libraries/nestjs-libraries/src/integrations/social.abstract.ts(2 hunks)libraries/nestjs-libraries/src/integrations/social/instagram.provider.ts(5 hunks)
💤 Files with no reviewable changes (1)
- apps/frontend/src/components/launches/providers/instagram/instagram.provider.tsx
🧰 Additional context used
🪛 GitHub Check: ESLint
apps/frontend/src/components/launches/providers/instagram/instagram.collaborators.tsx
[warning] 19-19: Disallow unused variables
'formState' is assigned a value but never used.
[warning] 19-19: Disallow unused variables
'control' is assigned a value but never used.
apps/frontend/src/components/launches/providers/instagram/instagram.tags.tsx
[warning] 22-22: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useCallback has missing dependencies: 'name' and 'onChange'. Either include them or remove the dependency array.
[warning] 34-34: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useCallback has missing dependencies: 'name' and 'onChange'. Either include them or remove the dependency array.
[warning] 42-42: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useEffect has missing dependencies: 'getValues' and 'props.name'. Either include them or remove the dependency array.
🔇 Additional comments (6)
apps/frontend/src/components/launches/add.edit.model.tsx (1)
279-280: Ensure correct handling of key.settings in validation
The addition of key.settings as a parameter enhances validation by incorporating specific settings. Verify that key.checkValidity correctly handles the settings parameter and that all implementations of checkValidity are updated accordingly.
libraries/nestjs-libraries/src/dtos/posts/providers-settings/instagram.dto.ts (1)
1-18: Well-defined DTO classes with appropriate validations
The Collaborators and InstagramDto classes are correctly defined with suitable validation decorators, ensuring data integrity and type safety.
libraries/nestjs-libraries/src/integrations/social.abstract.ts (1)
47-50: Review the enhanced error handling conditions
The updated condition in the fetch method refines error handling by excluding specific cases. Ensure that these additional checks for "Unsupported format" and '2207018' accurately capture the intended scenarios without affecting other error cases.
apps/frontend/src/components/launches/providers/show.all.providers.tsx (1)
10-10: LGTM! Import path updated correctly.
The import path update aligns with the new Instagram collaborators functionality.
apps/frontend/src/components/launches/providers/high.order.provider.tsx (2)
71-80: Verify integration points for settings validation.
The addition of the settings parameter to checkValidity requires verification of all integration points to ensure they properly pass settings objects.
Let's check the integration points:
71-80: Consider addressing type assertions and improving type safety.
The code contains @ts-ignore comments that could be replaced with proper type definitions. Additionally, verify that all consuming code properly types the settings parameter.
Let's check for usage patterns and potential type issues:
| .filter((f) => f.path.indexOf('mp4') > -1) | ||
| .flatMap((p) => p.path) | ||
| .map((p) => { |
There was a problem hiding this comment.
Correct the usage of flatMap and improve video file detection
The use of flatMap is unnecessary here since p.path is a string. It should be replaced with map. Additionally, detecting video files by checking if 'mp4' is in the path string is case-sensitive and may not cover all video formats. Consider using a regular expression to match video file extensions case-insensitively.
Apply this diff to correct the code:
- .filter((f) => f.path.indexOf('mp4') > -1)
+ .filter((f) => /\.(mp4|mov|avi)$/i.test(f.path))
- .flatMap((p) => p.path)
+ .map((p) => p.path)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .filter((f) => f.path.indexOf('mp4') > -1) | |
| .flatMap((p) => p.path) | |
| .map((p) => { | |
| .filter((f) => /\.(mp4|mov|avi)$/i.test(f.path)) | |
| .map((p) => p.path) | |
| .map((p) => { |
| const onDelete = useCallback( | ||
| (tagIndex: number) => { | ||
| const modify = tagValue.filter((_, i) => i !== tagIndex); | ||
| setTagValue(modify); | ||
| onChange({ target: { value: modify, name } }); | ||
| }, | ||
| [tagValue] | ||
| ); |
There was a problem hiding this comment.
Include missing dependencies in useCallback for onDelete
The onDelete function uses name and onChange, but they are not included in the dependency array of the useCallback hook. This omission may lead to stale references if name or onChange change. Include them in the dependency array to ensure consistency.
Apply this diff:
- [tagValue]
+ [tagValue, name, onChange]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onDelete = useCallback( | |
| (tagIndex: number) => { | |
| const modify = tagValue.filter((_, i) => i !== tagIndex); | |
| setTagValue(modify); | |
| onChange({ target: { value: modify, name } }); | |
| }, | |
| [tagValue] | |
| ); | |
| const onDelete = useCallback( | |
| (tagIndex: number) => { | |
| const modify = tagValue.filter((_, i) => i !== tagIndex); | |
| setTagValue(modify); | |
| onChange({ target: { value: modify, name } }); | |
| }, | |
| [tagValue, name, onChange] | |
| ); |
🧰 Tools
🪛 GitHub Check: ESLint
[warning] 22-22: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useCallback has missing dependencies: 'name' and 'onChange'. Either include them or remove the dependency array.
| const onAddition = useCallback( | ||
| (newTag: any) => { | ||
| if (tagValue.length >= 3) { | ||
| return; | ||
| } | ||
| const modify = [...tagValue, newTag]; | ||
| setTagValue(modify); | ||
| onChange({ target: { value: modify, name } }); | ||
| }, | ||
| [tagValue] | ||
| ); |
There was a problem hiding this comment.
Include missing dependencies in useCallback for onAddition
The onAddition function uses name and onChange, but they are not included in the dependency array of the useCallback hook. This could cause issues if name or onChange update. Add them to the dependency array to maintain proper references.
Apply this diff:
- [tagValue]
+ [tagValue, name, onChange]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onAddition = useCallback( | |
| (newTag: any) => { | |
| if (tagValue.length >= 3) { | |
| return; | |
| } | |
| const modify = [...tagValue, newTag]; | |
| setTagValue(modify); | |
| onChange({ target: { value: modify, name } }); | |
| }, | |
| [tagValue] | |
| ); | |
| const onAddition = useCallback( | |
| (newTag: any) => { | |
| if (tagValue.length >= 3) { | |
| return; | |
| } | |
| const modify = [...tagValue, newTag]; | |
| setTagValue(modify); | |
| onChange({ target: { value: modify, name } }); | |
| }, | |
| [tagValue, name, onChange] | |
| ); |
🧰 Tools
🪛 GitHub Check: ESLint
[warning] 34-34: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useCallback has missing dependencies: 'name' and 'onChange'. Either include them or remove the dependency array.
| useEffect(() => { | ||
| const settings = getValues()[props.name]; | ||
| if (settings) { | ||
| setTagValue(settings); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Include missing dependencies in useEffect
The useEffect hook uses getValues and name, but these are not included in the dependency array. To ensure the effect runs correctly when these values change, include them in the dependency array.
Apply this diff:
- }, []);
+ }, [getValues, name]);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: ESLint
[warning] 42-42: verifies the list of dependencies for Hooks like useEffect and similar
React Hook useEffect has missing dependencies: 'getValues' and 'props.name'. Either include them or remove the dependency array.
| checkValidity?: ( | ||
| value: Array<Array<{ path: string }>>, | ||
| settings: any | ||
| ) => Promise<string | true>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a generic type parameter instead of any for better type safety.
The settings parameter using any type could lead to runtime errors. Consider using a generic type parameter to maintain type safety across different integrations.
- checkValidity?: (
- value: Array<Array<{ path: string }>>,
- settings: any
- ) => Promise<string | true>;
+ checkValidity?: <T>(
+ value: Array<Array<{ path: string }>>,
+ settings: T
+ ) => Promise<string | true>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checkValidity?: ( | |
| value: Array<Array<{ path: string }>>, | |
| settings: any | |
| ) => Promise<string | true>; | |
| checkValidity?: <T>( | |
| value: Array<Array<{ path: string }>>, | |
| settings: T | |
| ) => Promise<string | true>; |
| checkValidity?: ( | ||
| value: Array<Array<{ path: string }>>, | ||
| settings: any | ||
| ) => Promise<string | true>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same generic type parameter here for consistency.
For consistency with the type definition above, apply the same generic type parameter here.
- checkValidity?: (
- value: Array<Array<{ path: string }>>,
- settings: any
- ) => Promise<string | true>,
+ checkValidity?: <T>(
+ value: Array<Array<{ path: string }>>,
+ settings: T
+ ) => Promise<string | true>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checkValidity?: ( | |
| value: Array<Array<{ path: string }>>, | |
| settings: any | |
| ) => Promise<string | true>, | |
| checkValidity?: <T>( | |
| value: Array<Array<{ path: string }>>, | |
| settings: T | |
| ) => Promise<string | true>, |
| ? isStory | ||
| ? `video_url=${m.url}&media_type=STORIES` | ||
| : `video_url=${m.url}&media_type=REELS` | ||
| : isStory | ||
| ? `video_url=${m.url}&media_type=STORIES` | ||
| : `video_url=${m.url}&media_type=VIDEO` | ||
| : isStory | ||
| ? `image_url=${m.url}&media_type=STORIES` | ||
| : `image_url=${m.url}`; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor complex media type logic for better readability.
The nested ternary operations make the code hard to read and maintain. Consider extracting this logic into a separate function.
+ private getMediaType(url: string, isStory: boolean, isMultiMedia: boolean): string {
+ const isVideo = url.indexOf('.mp4') > -1;
+ if (!isVideo) {
+ return isStory ? `image_url=${url}&media_type=STORIES` : `image_url=${url}`;
+ }
+ if (isStory) {
+ return `video_url=${url}&media_type=STORIES`;
+ }
+ if (!isMultiMedia) {
+ return `video_url=${url}&media_type=REELS`;
+ }
+ return `video_url=${url}&media_type=VIDEO`;
+ }Then use it in the code:
- const mediaType =
- m.url.indexOf('.mp4') > -1
- ? firstPost?.media?.length === 1
- ? isStory
- ? `video_url=${m.url}&media_type=STORIES`
- : `video_url=${m.url}&media_type=REELS`
- : isStory
- ? `video_url=${m.url}&media_type=STORIES`
- : `video_url=${m.url}&media_type=VIDEO`
- : isStory
- ? `image_url=${m.url}&media_type=STORIES`
- : `image_url=${m.url}`;
+ const mediaType = this.getMediaType(m.url, isStory, firstPost?.media?.length > 1);Committable suggestion skipped: line range outside the PR's diff.
| music(accessToken: string, data: { q: string }) { | ||
| return this.fetch( | ||
| `https://graph.facebook.com/v20.0/music/search?q=${encodeURIComponent( | ||
| data.q | ||
| )}&access_token=${accessToken}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to the music method.
The music method should include error handling and type safety.
- music(accessToken: string, data: { q: string }) {
- return this.fetch(
- `https://graph.facebook.com/v20.0/music/search?q=${encodeURIComponent(
- data.q
- )}&access_token=${accessToken}`
- );
+ async music(accessToken: string, data: { q: string }): Promise<any> {
+ try {
+ const response = await this.fetch(
+ `https://graph.facebook.com/v20.0/music/search?q=${encodeURIComponent(
+ data.q
+ )}&access_token=${accessToken}`
+ );
+ return response.json();
+ } catch (error) {
+ throw new Error(`Failed to search music: ${error.message}`);
+ }
}Committable suggestion skipped: line range outside the PR's diff.
instagram stories, add collaborators, infrastructure for settings in validation
Summary by CodeRabbit
New Features
InstagramCollaboratorscomponent for managing Instagram post types and collaborators.InstagramCollaboratorsTagscomponent for tagging collaborators with a maximum limit.Improvements
Bug Fixes