-
Notifications
You must be signed in to change notification settings - Fork 1
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
merge main into release #579
Conversation
…-devdependencies chore(deps): update devdependencies (patch)
…-nextjs-monorepo fix(deps): update dependency next to v14.2.19
…-nextjs-monorepo fix(deps): update dependency next to v14.2.20
…-devdependencies chore(deps): update dependency eslint-config-next to v14.2.20
…elemetry-js-monorepo fix(deps): update opentelemetry-js monorepo to ^0.56.0 (minor)
… docker tag to v9.5-1733824671
…try.access.redhat.com-ubi9-nodejs-20-minimal-9.5.x chore(deps): update registry.access.redhat.com/ubi9/nodejs-20-minimal docker tag to v9.5-1733824671
… docker tag to v9.5-1734309067
…try.access.redhat.com-ubi9-nodejs-20-minimal-9.5.x chore(deps): update registry.access.redhat.com/ubi9/nodejs-20-minimal docker tag to v9.5-1734309067
feat: improve external service integration
…i-specifications chore: update open-api-specifications
…-devdependencies chore(deps): update dependency tailwindcss to v3.4.17
Snyk has created this PR to upgrade tailwind-merge from 2.5.3 to 2.5.5. See this package in npm: tailwind-merge See this project in Snyk: https://app.snyk.io/org/brunopacheco1/project/d90a5e56-2020-4abe-8f3a-101f4e46d57b?utm_source=github&utm_medium=referral&page=upgrade-pr
Snyk has created this PR to upgrade cmdk from 1.0.0 to 1.0.4. See this package in npm: cmdk See this project in Snyk: https://app.snyk.io/org/brunopacheco1/project/d90a5e56-2020-4abe-8f3a-101f4e46d57b?utm_source=github&utm_medium=referral&page=upgrade-pr
Snyk has created this PR to upgrade class-variance-authority from 0.7.0 to 0.7.1. See this package in npm: class-variance-authority See this project in Snyk: https://app.snyk.io/org/brunopacheco1/project/d90a5e56-2020-4abe-8f3a-101f4e46d57b?utm_source=github&utm_medium=referral&page=upgrade-pr
… docker tag to v9.5-1734514731
…try.access.redhat.com-ubi9-nodejs-20-minimal-9.5.x chore(deps): update registry.access.redhat.com/ubi9/nodejs-20-minimal docker tag to v9.5-1734514731
…2929b7a6fe71756ca9ec645582a48f5 [Snyk] Upgrade class-variance-authority from 0.7.0 to 0.7.1
…a0715511e4452711f7df9a89d62f69e [Snyk] Upgrade cmdk from 1.0.0 to 1.0.4
…234289bf1d994647ebf22e23f28ccfc [Snyk] Upgrade tailwind-merge from 2.5.3 to 2.5.5
…portal-frontend-missing-form-validation fix: ART-12382/missing form validation
…u for medium screens
…all screen sizes)
…portal-frontend-custom-text-and-links-are-not-displayed fix: ART-12303/custom text not displayed
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
…elemetry-auto-instrumentations-node-0.x fix(deps): update dependency @opentelemetry/auto-instrumentations-node to ^0.56.0
…ers if less than 3 names
…ove-header-small-screens Art 12300 improve header small screens
Co-authored-by: Kacem Bechka <[email protected]>
… docker tag to v9.5-1738870241
Reviewer's Guide by SourceryThis pull request merges changes from main into the release branch. The implementation mainly consists of synchronizing updates across the application by refactoring type usages, updating API integrations and error handling, revising UI components for better usability and responsiveness, and modifying configuration files (Dockerfile, package.json, GitHub Workflows). In addition, extensive modifications were made to tests and OpenAPI specifications, ensuring the updated APIs and new types (migrating from legacy types to new ones from the /app/api directories) are used consistently throughout the codebase. Class diagram for Revised Application Data TypesclassDiagram
class RetrievedApplication {
+number id
+ApplicationState state
+RetrievedApplicationForm[] forms
+Event[] events
+License[] licenses
+Attachment[] attachments
}
class RetrievedApplicationForm {
+number id
+Label[] externalTitle
+RetrievedApplicationFormField[] fields
}
class RetrievedApplicationFormField {
+number id
+FormFieldType type
+string value
+FormFieldTableValue[][] tableValues
+FormFieldOption[] options
}
class FormFieldOption {
+string key
+Label[] label
}
class Label {
+string language
+string name
}
%% Relationships
RetrievedApplication --> RetrievedApplicationForm : contains
RetrievedApplicationForm --> RetrievedApplicationFormField : contains
RetrievedApplicationFormField --> FormFieldOption : uses
FormFieldOption --> Label : uses
RetrievedApplicationForm --> Label : externalTitle uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @brunopacheco1 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Great overall refactoring and type updates; consider replacing repetitive non-null assertions (!) with proper type guards or runtime checks to improve robustness.
- Verify that all API client changes are thoroughly integrated tested to catch any runtime issues due to type shifting and new module paths.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -1,7 +1,7 @@ | |||
# SPDX-FileCopyrightText: 2024 PNED G.I.E. | |||
# | |||
# SPDX-License-Identifier: Apache-2.0 | |||
FROM registry.access.redhat.com/ubi9/nodejs-20-minimal:9.5-1732617235 AS base | |||
FROM registry.access.redhat.com/ubi9/nodejs-22-minimal:9.5-1738870241 AS base |
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.
question (performance): Updated base image to Node.js 22.
Switching from Node.js 20 to Node.js 22 may provide performance and security improvements. Please confirm that all runtime dependencies and build scripts are compatible with Node.js 22.
@@ -30,8 +30,8 @@ export function createDatasetCardItems(dataset: SearchedDataset): CardItem[] { | |||
}, | |||
{ | |||
text: | |||
(dataset.publishers?.length > 0 && | |||
`Published by ${dataset.publishers.map((p) => p.name).join(", ")}`) || | |||
(dataset.publishers!.length > 0 && |
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.
question (bug_risk): Check non-null assertion for publisher data.
Using the non-null assertion on 'dataset.publishers' signals that the value is expected to be present, but verify that this is guaranteed. If not, a fallback or conditional rendering might prevent potential runtime errors.
By using this website, users agree to abide by these website terms (in their most recent version, as available on the website). GDI Coordination reserves the right to modify this website and the website terms from time to time. | ||
The data catalogue is operated as an online platform which allows its users to easily find diverse collection of datasets across various themes and organisations. To access the datasets the user must contact directly the relevant “Contact point”. The user may export the metadata of the dataset in the following formats: RDF, TTL, JSON-LD. | ||
|
||
By using this website users agree to abide by these website terms (in their most recent version, as available on the website). The LNDS reserves the right to modify this website and the website terms from time to time. |
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.
suggestion (typo): Add a comma after "website" for better readability.
It should read "By using this website, users agree..."
By using this website users agree to abide by these website terms (in their most recent version, as available on the website). The LNDS reserves the right to modify this website and the website terms from time to time. | |
By using this website, users agree to abide by these website terms (in their most recent version, as available on the website). The LNDS reserves the right to modify this website and the website terms from time to time. |
**Essential (Functional) cookies**: | ||
| Cookies | Description | Duration | | ||
|---------|-------------|----------| | ||
| **Host-next-auth.csrf-token; **Secure-next-auth.callback-url; **Secure-next-auth.pkce.code_verifier; **Secure-next-auth.state | User authentication (Next.js auth options) | While the browser is open | |
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.
suggestion (typo): Remove the extra space between "Host-next-auth.csrf-token;" and "**Secure-next-auth.callback-url".
| **Host-next-auth.csrf-token; **Secure-next-auth.callback-url; **Secure-next-auth.pkce.code_verifier; **Secure-next-auth.state | User authentication (Next.js auth options) | While the browser is open | | |
| **Host-next-auth.csrf-token;** **Secure-next-auth.callback-url;** **Secure-next-auth.pkce.code_verifier;** **Secure-next-auth.state | User authentication (Next.js auth options) | While the browser is open | |
const tokenDecrypted = decrypt(session[tokenType]); | ||
return tokenDecrypted; |
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.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const tokenDecrypted = decrypt(session[tokenType]); | |
return tokenDecrypted; | |
return decrypt(session[tokenType]); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
export function getInitials(name?: string) { | ||
if (!name) return null; |
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.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces
)
if (!name) return null; | |
if (!name) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
Summary by Sourcery
Merge the main branch into the release branch, introducing a new mobile-friendly navigation bar, refactoring the application provider for better error handling, and updating the header component for dynamic navigation item generation. Enhance dataset search and export functionalities, update the Dockerfile and CI workflows, and improve documentation and tests. Remove deprecated API routes and services.
New Features:
Enhancements:
Build:
CI:
Deployment:
Documentation:
Tests:
Chores: