-
-
Notifications
You must be signed in to change notification settings - Fork 976
chore: add content with case studies #4486
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
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReplaces the adopters pipeline with a usecases pipeline: adds Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI / build (scripts/index.ts)
participant Usecases as buildUsecasesList()
participant Helper as writeJSON()
participant RefParser as $RefParser
participant FS as Filesystem
CI->>Usecases: await buildUsecasesList()
Usecases->>Helper: writeJSON(read: config/usecases.yaml, write: config/usecases.json, dereference: true)
Helper->>RefParser: dereference(parsed JSON)
RefParser-->>Helper: dereferenced JSON
Helper->>FS: write config/usecases.json (dereferenced)
FS-->>CI: file written
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4486 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 780 +2
Branches 144 144
=========================================
+ Hits 778 780 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4486--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config/usecases.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test NodeJS PR - macos-13
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
config/usecases.yaml (1)
105-114: Normalize content type field values.Adeo (line 108) and HDI Global (line 113) use the type
Use Case, which differs stylistically from other entries (Video, Slides, Article, Docs, Code). For consistency and downstream parsing, consider renaming to a standard type or documenting the full list of allowed values.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
config/adopters.yml(0 hunks)config/usecases.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- config/adopters.yml
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
config/usecases.yaml (2)
50-59: Verify duplicate URL for Walmart and eBay.Walmart (line 54) and eBay (line 59) both reference the same YouTube URL:
https://www.youtube.com/watch?v=SxTpGRaNIPo. Confirm whether this is intentional or a copy-paste error. If incorrect, correct the eBay entry.
1-23: Approved: Use cases section is well-structured.The use cases are clearly defined with consistent formatting and meaningful descriptions. The governance spelling is now correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/helpers/readAndWriteJson.ts (1)
16-41: Fix the unconditional dereferencing logic.The
dereferenceparameter is accepted but never used. Line 34 unconditionally dereferences the JSON content regardless of the parameter value, which doesn't match the intended behavior.Apply this diff to make dereferencing conditional:
- const dereferenced = await $RefParser.dereference(jsonContent); + let outputContent = jsonContent; + + if (dereference) { + try { + outputContent = await $RefParser.dereference(jsonContent); + } catch (err) { + return Promise.reject(err); + } + } // Attempt to write the JSON content to file try { - await writeFile(writePath, JSON.stringify(dereferenced)); + await writeFile(writePath, JSON.stringify(outputContent)); } catch (err) { return Promise.reject(err); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.gitignore(1 hunks)config/usecases.yaml(1 hunks)package.json(1 hunks)pages/casestudies/index.tsx(5 hunks)scripts/adopters/index.ts(0 hunks)scripts/helpers/readAndWriteJson.ts(3 hunks)scripts/index.ts(2 hunks)scripts/usecases/index.ts(1 hunks)tests/index.test.ts(3 hunks)tests/usecases/index.test.ts(1 hunks)
💤 Files with no reviewable changes (1)
- scripts/adopters/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- config/usecases.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-01T09:55:20.531Z
Learnt from: akshatnema
PR: asyncapi/website#3101
File: tests/build-rss.test.js:25-27
Timestamp: 2024-11-01T09:55:20.531Z
Learning: In `tests/build-rss.test.js`, replacing `jest.resetModules()` with `jest.resetAllMocks()` in the `afterEach()` block causes errors. It is necessary to use `jest.resetModules()` to reset the module registry between tests in this file.
Applied to files:
tests/index.test.ts
🧬 Code graph analysis (4)
scripts/usecases/index.ts (1)
scripts/helpers/readAndWriteJson.ts (1)
writeJSON(16-44)
scripts/index.ts (1)
scripts/usecases/index.ts (1)
buildUsecasesList(18-20)
tests/usecases/index.test.ts (1)
scripts/usecases/index.ts (1)
buildUsecasesList(18-20)
tests/index.test.ts (1)
scripts/usecases/index.ts (1)
buildUsecasesList(18-20)
🔇 Additional comments (15)
package.json (1)
126-126: LGTM!The addition of
@apidevtools/json-schema-ref-parseras a devDependency is properly specified and aligns with its usage in the dereferencing logic introduced inscripts/helpers/readAndWriteJson.ts..gitignore (1)
10-10: LGTM!The update to ignore
config/usecases.jsoninstead ofconfig/adopters.jsoncorrectly reflects the migration from adopters to usecases.scripts/index.ts (2)
5-5: LGTM!The import path correctly references the new usecases module.
36-36: LGTM!The function call correctly invokes the new
buildUsecasesList()function, maintaining the build sequence.pages/casestudies/index.tsx (7)
12-12: LGTM!The import correctly references the new
usecases.jsonconfiguration file.
15-24: LGTM!The interface definitions correctly reflect the new data model, with
Resourceusingtypeandurl, andAdopterusingnameanddescription. These changes are consistently applied throughout the component.
34-34: LGTM!The construction of
AdoptersListfromUsecasesList.datacorrectly adapts to the new data structure.
72-72: LGTM!Using
UsecasesList.descriptionfor the adopters section description is a good approach, making the content configurable rather than hardcoded.
76-76: LGTM!The PR submission link correctly points to the new
usecases.ymlconfiguration file.
98-114: LGTM!The desktop table view correctly uses the new field names (
entry.name,entry.description,resource.type,resource.url) consistently with the updated data model.
120-148: LGTM!The mobile card view correctly mirrors the desktop implementation, using the updated field names consistently throughout.
scripts/usecases/index.ts (1)
1-20: LGTM!The
buildUsecasesList()function is well-documented and correctly delegates towriteJSONwith the appropriate parameters, including thedereferenceflag set totrue. The path resolution usingresolveandcurrentDirPathis correct.tests/index.test.ts (3)
3-3: LGTM!The import correctly references the new
buildUsecasesListfunction from the usecases module.
13-13: LGTM!The mock path correctly references the new usecases module.
34-34: LGTM!The test expectation correctly verifies that
buildUsecasesListis called during the start sequence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
config/usecases.yaml(1 hunks)pages/casestudies/index.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pages/casestudies/index.tsx (1)
components/typography/Paragraph.tsx (1)
Paragraph(28-56)
🔇 Additional comments (4)
pages/casestudies/index.tsx (4)
15-24: LGTM! Interface definitions align with new data model.The updated
ResourceandAdopterinterfaces correctly reflect the structure defined inconfig/usecases.yaml. The transition from{title, link}to{type, url}and from{companyName, useCase}to{name, description}is consistent throughout the file.
34-34: LGTM! Data extraction correctly maps UsecasesList to AdoptersList.The use of
Object.values(UsecasesList.data)correctly extracts all company/organization entries from the dereferenced usecases JSON for rendering in the adopters table.
98-116: LGTM! Desktop table rendering correctly uses new field names.The table rows correctly map
entry.name,entry.description,resource.type, andresource.urlfor all adopter entries. The rendering logic is consistent with the updated interface definitions.
120-148: LGTM! Mobile card view correctly uses new field names.The mobile card layout consistently renders
entry.name,entry.description,resource.type, andresource.url. The responsive design maintains parity with the desktop table view.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks clean. Thanks @derberg
|
/rtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pages/casestudies/index.tsx (2)
98-116: Consider adding an empty state message.The table correctly renders the adopters list with the updated property names. However, when
AdoptersListis empty, the table displays only headers with no rows, which might confuse users.Consider adding an empty state:
{AdoptersList.length === 0 ? ( <tr> <td colSpan={3} className='border-2 p-4 text-center text-gray-500'> No adopters available at this time. </td> </tr> ) : ( AdoptersList.map((entry: Adopter, index: number) => ( // ... existing row rendering )) )}
120-148: Mobile card rendering looks good; consider empty state.The mobile view correctly uses the new property names and properly structures the card layout.
Similar to the desktop table, you could add an empty state for better UX:
{AdoptersList.length === 0 ? ( <div className='text-center p-8 text-gray-500'> No adopters available at this time. </div> ) : ( AdoptersList.map((entry: Adopter, index: number) => ( // ... existing card rendering )) )}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/casestudies/index.tsx(5 hunks)tests/usecases/index.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/usecases/index.test.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: automerge-for-humans
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (1)
pages/casestudies/index.tsx (1)
15-24: LGTM!The interface definitions are clear and properly reflect the new data structure from the usecases configuration.
| const image: string = '/img/social/case-studies.webp'; | ||
| const title: string = 'Case Studies'; | ||
|
|
||
| const AdoptersList = Object.values(UsecasesList.data) as Adopter[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add defensive checks for data access.
The code accesses UsecasesList.data without verifying it exists, and uses a type assertion that bypasses compile-time validation. If the JSON structure is missing or malformed, this will cause runtime errors.
Apply this diff to add defensive checks:
- const AdoptersList = Object.values(UsecasesList.data) as Adopter[];
+ const AdoptersList = (UsecasesList?.data ? Object.values(UsecasesList.data) : []) as Adopter[];Or, for more explicit handling:
- const AdoptersList = Object.values(UsecasesList.data) as Adopter[];
+ const AdoptersList: Adopter[] = [];
+ if (UsecasesList?.data && typeof UsecasesList.data === 'object') {
+ AdoptersList.push(...(Object.values(UsecasesList.data) as Adopter[]));
+ }📝 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 AdoptersList = Object.values(UsecasesList.data) as Adopter[]; | |
| const AdoptersList = (UsecasesList?.data ? Object.values(UsecasesList.data) : []) as Adopter[]; |
| const AdoptersList = Object.values(UsecasesList.data) as Adopter[]; | |
| const AdoptersList: Adopter[] = []; | |
| if (UsecasesList?.data && typeof UsecasesList.data === 'object') { | |
| AdoptersList.push(...(Object.values(UsecasesList.data) as Adopter[])); | |
| } |
🤖 Prompt for AI Agents
In pages/casestudies/index.tsx around line 34, the line "const AdoptersList =
Object.values(UsecasesList.data) as Adopter[]" assumes UsecasesList and
UsecasesList.data always exist and uses a forced type assertion; change this to
defensively check UsecasesList and UsecasesList.data (e.g., guard with
null/undefined checks and Array.isArray/Object.keys as appropriate), fallback to
an empty array when data is missing, and remove the unchecked "as Adopter[]"
assertion by applying a small runtime type guard or mapping/validation of each
item to the Adopter shape so the resulting AdoptersList is a properly validated
array.
| <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'> | ||
| {UsecasesList.description} | ||
| </Paragraph> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null check for description property.
The code renders UsecasesList.description without checking if it exists. If the description field is missing from the JSON, this could display "undefined" or cause rendering issues.
Apply this diff to add a conditional check:
- <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'>
- {UsecasesList.description}
- </Paragraph>
+ {UsecasesList?.description && (
+ <Paragraph typeStyle={ParagraphTypeStyle.md} className='my-4 max-w-4xl'>
+ {UsecasesList.description}
+ </Paragraph>
+ )}🤖 Prompt for AI Agents
In pages/casestudies/index.tsx around lines 80 to 82, the component directly
renders UsecasesList.description which can be undefined; update the render to
guard against null/undefined by checking the property before rendering (e.g.,
render UsecasesList.description only if it exists, or use a safe fallback like
an empty string or a default message). Ensure the Paragraph receives a string
(use optional chaining or a nullish coalescing fallback) so nothing like
"undefined" is displayed.
|
/rtm |
no more adopters, now we have one files with use cases and companies that do these use cases.
ui and tests aligned.
this is critical pr for @Shriya-Chauhan and new things she's doing to landing page
@Shriya-Chauhan you have 7 use cases but on landing page 6 are completely enough
old: https://www.asyncapi.com/casestudies
new: https://deploy-preview-4486--asyncapi-website.netlify.app/casestudies
Summary by CodeRabbit
New Features
Refactor
Chores