-
-
Notifications
You must be signed in to change notification settings - Fork 102
Fix Recent Releases Card: Show Tag as Fallback and Correct Release URLs #1566
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
base: main
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe RecentReleases component's link href logic was changed from using a fallback URL Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Graph Analysis (1)frontend/src/components/RecentReleases.tsx (2)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate Unit Tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
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: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/RecentReleases.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/RecentReleases.tsx (1)
frontend/src/components/TruncatedText.tsx (1)
TruncatedText
(3-45)
🔇 Additional comments (1)
frontend/src/components/RecentReleases.tsx (1)
78-78
: LGTM! Proper fallback implementation.The fallback logic correctly displays the tag name when the release name is missing, which aligns perfectly with the PR objectives. The use of optional chaining ensures safe property access.
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.
Thanks for working on this.
One big issue I found ⬇️
@rishyym0927 Hi! Any updates on this one? |
@kasya can you clarify one thing, that if the url is not present then what should I use in replacement of that or how to deal with it? Also what if the tag name or title name both are missing how can we deal with it then? shouldn't we make these fields mandatory at the backend part ? |
@rishyym0927 the item you work with doesn't have a I would recommend you to check how links for releases work on the organization page. It's the same component. |
Also, tests are failing now. Please run |
Hey @kasya as I can see that the graphql query return the url in recentReleases, so it does have the url property in it. What goes wrong is I gues that sometimes the when url link is not received then it basically redirect to home page of the nest where as we shoudl redirect it not found page. IF im correct then can I mplement this solution cause I dont think there is any need of making changes in hte backend as of now
Hey @kasya, I checked the GraphQL query, and it looks like the recentReleases does return the url property. From what I observed, the issue seems to occur when the url is not present for a particular release — in such cases, it redirects to the home page of Nest. Ideally, we should redirect to a 404 or a "Not Found" page instead. If my understanding is correct, would it make sense for me to implement this behavior on the frontend? I believe this might be sufficient for now, without requiring changes on the backend — but happy to discuss further if you think otherwise. Let me know what you think! |
I implemented a new solution which works pretty well, even if the url fiield is missing @kasya |
@rishyym0927 have you compared the data on those 2 pages? The data that is sent to the project page releases does have the But you don't have it in the objects you work with on the repo page. GraphQL only shows you what's available - not necessarily all of it was sent to the frontend. And that's exactly the case for this page. |
Hey @kasya, You're absolutely right — I hadn’t compared the data between the project page and the repo page closely enough. I now see that while the GraphQL query can return the url, it isn’t being passed to the frontend in the repo page context, which is likely why the redirection issue occurs there. Apologies for the oversight on my part. I’ll dig into this further |
|
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.
I cleaned this up a bit (removed prev.added logic for working with dates that was not needed, we have a formatDate()
util that takes care of that).
Also pushed missing part with getting url
for a release in repository query.
This works fine now! 👌🏼
This PR improves the Recent Releases card component with the following changes:
Fixes Improve recent releases card #1537
Ensures that each release item displays a readable name: if the release name is absent, the tagName is shown as a fallback.
Fixes the release link: if the url is missing or invalid, the link now correctly points to the organization's repository release page using the organization, repository, and tag.
No changes are required in the backend as long as all necessary fields (name, tagName, url, organizationName, repositoryName) are present in the API response.
These changes address issues where releases from external organizations were missing information or had incorrect links. No unrelated code was modified.