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

[NEW] Create releases tab in the marketplace app info page #25965

Merged
merged 17 commits into from
Jun 29, 2022

Conversation

rique223
Copy link
Contributor

@rique223 rique223 commented Jun 22, 2022

Proposed changes (including videos or screenshots)

Added a Releases tab to the app info page of installed marketplace apps. This tab will show all the released versions of a given app with its version number, release date in humanized form, and the changelog of this given release with the information provided by the publisher, this changelog accepts and renders markdown. Also refactored some component names and logic for maintainability reasons.
Demo gif:
app-releases-tab-final

Issue(s)

Steps to test or reproduce

Further comments

…app info page

Refactored the ApiDisplay to be inside of the AppDetails component, changed the name of the AppDetailsPageContent component since now it is a tab content, changed the name of the AppLogsPage for the same reason and also changed the name of the AppSecurity page component.
rique223 and others added 11 commits June 23, 2022 15:56
Created new components AppReleases and ReleaseItem and mocked some data to make it faithful to figma. Will be integrating it with the back-end next.
Implemented the first necessary steps to integrating the AppReleases component with the back-end by passing down the appId and creating a local hook to deal with fetching the versions. Unfortunately the GET request necessary to fetch that data does not exist yet. Will be implementing it after the request creation.
Created a apps/:appId/versions endpoint to fetch all versions of a given app so that I finish the AppReleases component.

Co-authored-by: Matheus Carmo <matheus.carmo@rocket.chat>
Fetched the app versions list, mapped it into a releases array, and implemented the AccordionItems list of releases.
Refactored the timestamp of the last updated field of the AppDetailsHeader component for maintainability reasons and to make it more straightforward.
Implemented the useTimeAgo hook to format the created date of the release entries. Also removed the Time normalizer sufix from the date strings to make it respect the current selected locale.
Refactored the i18n english dictionary to solve the thousands of unwanted modifications on remote.
Implemented a parsing method, using dangerouslySetInnerHTML, to parse the html of the release item rendered markdown. Also refactored some of the loading and error logic for the AppReleases to better benefit from the async state nature of useEndpointData.
@rique223 rique223 marked this pull request as ready for review June 27, 2022 19:28
@rique223 rique223 requested review from a team as code owners June 27, 2022 19:28
Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please have it only load the release tab and not cause the entire page to seem like it is loading? I expect to have it only load the tab information whenever I click on a tab when that app information is already there.

Fixed a problem where the whole details page component would refresh upon clicking on a tab if you came from the marketplace list of apps.
@rique223
Copy link
Contributor Author

Can we please have it only load the release tab and not cause the entire page to seem like it is loading? I expect to have it only load the tab information whenever I click on a tab when that app information is already there.

This was a little bug because of a misuse of the router functions, should be solved after this last commit.

@graywolf336
Copy link
Contributor

Awesome, can you update the demo gif?

@rique223
Copy link
Contributor Author

Awesome, can you update the demo gif?

Sure can do

@rique223 rique223 requested review from graywolf336 and ggazzo June 28, 2022 16:14
ggazzo
ggazzo previously approved these changes Jun 28, 2022
@ggazzo ggazzo added this to the 5.0.0 milestone Jun 28, 2022
@ggazzo ggazzo added stat: ready to merge PR tested and approved waiting for merge labels Jun 28, 2022
Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When no update information has been provided, let's provide a generic text saying "no release information was provided". You can get with the designers to see what they think would be better.

@kodiakhq kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Jun 29, 2022
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Jun 29, 2022

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

Created a fallback for when a release entry has no changelog. Now it shows a "No release information provided" text on the accordion item.
graywolf336
graywolf336 previously approved these changes Jun 29, 2022
Copy link
Contributor

@graywolf336 graywolf336 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@ggazzo ggazzo added the stat: ready to merge PR tested and approved waiting for merge label Jun 29, 2022
@rique223 rique223 requested a review from ggazzo June 29, 2022 22:23
@kodiakhq kodiakhq bot merged commit 8486202 into develop Jun 29, 2022
@kodiakhq kodiakhq bot deleted the feat/marketplace-app-info-releases-tab branch June 29, 2022 22:52
@murtaza98 murtaza98 mentioned this pull request Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants