Skip to content

Comments

Discover works alert#498

Merged
bklaing2 merged 13 commits intomasterfrom
discover-works-alert
May 19, 2025
Merged

Discover works alert#498
bklaing2 merged 13 commits intomasterfrom
discover-works-alert

Conversation

@bklaing2
Copy link
Member

@bklaing2 bklaing2 commented May 15, 2025

Purpose

Adds an alert banner to the top of all pages when a User is signed in.

image

closes: https://github.com/datacite/product-backlog/issues/267

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

Summary by CodeRabbit

  • New Features

    • Introduced a custom-styled DataCiteButton component for consistent button appearance and icon support.
    • Added a DiscoverWorksAlert component to encourage users to add works to their ORCID profile, visible to signed-in users.
  • Refactor

    • Replaced inline example text and links with specialized components for works, organizations, people, and repositories, improving clarity and consistency.
    • Updated multiple components to use the new DataCiteButton for a unified button style.
    • Simplified conditional rendering for example searches across several search pages.
    • Refactored Pager component to use DataCiteButton and simplified URL logic.
  • Style

    • Added and updated SCSS modules for new button and alert components, and improved responsiveness.
    • Adjusted header and navigation spacing for better layout.
    • Removed obsolete button and pagination styles; added a new secondary color class.
  • Bug Fixes

    • Added runtime checks to ensure user session presence before rendering user-specific navigation options.
  • Chores

    • Removed outdated test cases for searching with no query.
    • Added an accent color constant for consistent theming.

@bklaing2 bklaing2 self-assigned this May 15, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 15, 2025

Walkthrough

The changes introduce new UI components for example text and alerts, refactor example and button components for modularity and styling, and update test cases and layout to use these new components. Several files are updated to simplify conditional rendering, enforce user session checks, and improve button and pagination UI consistency, with associated style and type improvements.

Changes

File(s) Change Summary
cypress/e2e/searchOrganization.test.ts
cypress/e2e/searchPerson.test.ts
cypress/e2e/searchWork.test.ts
Removed the "search no query" test case from each test suite; made minor formatting adjustments in export statements.
src/app/doi.org/page.tsx
src/app/orcid.org/page.tsx
src/app/page.tsx
src/app/repositories/page.tsx
src/app/ror.org/page.tsx
Refactored conditional rendering for empty queries to use new example text components; removed inline JSX and simplified logic.
src/app/layout.tsx Added the DiscoverWorksAlert component to the main layout, positioned below the header.
src/components/DataCiteButton/DataCiteButton.module.scss
src/components/DataCiteButton/DataCiteButton.tsx
Added a new styled button component (DataCiteButton) and its SCSS module, supporting variants, outline, icons, and link integration.
src/components/DiscoverWorksAlert/DiscoverWorksAlert.module.scss
src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx
Introduced a new alert component (DiscoverWorksAlert) with dismissible toast UI, session check, and action buttons; added corresponding styles.
src/components/DownloadMetadata/DownloadMetadata.tsx Replaced react-bootstrap/Button with DataCiteButton for modal and trigger buttons; updated styling and props.
src/components/ExampleText/ExampleText.tsx Removed the generic ExampleText default export; added specialized named exports for works, organizations, people, and repositories example text, each with headings and example links.
src/components/FacetList/FacetListGroup.module.scss Added !important to most CSS custom properties for higher specificity; no value changes.
src/components/ForceDirectedGraph/ForceDirectedGraph.tsx
src/components/SankeyGraph/SankeyGraph.tsx
Added TypeScript @ts-expect-error comments before <Vega> components to suppress errors; minor formatting changes.
src/components/Header/ClientButtons.tsx
src/components/Header/Dropdown.tsx
Added explicit user session checks; throw errors if user is not signed in before proceeding.
src/components/Header/Header.tsx Adjusted navigation layout classes for spacing and margin improvements.
src/components/Header/ServerButtons.tsx Updated sign-in button to use DataCiteButton via NavLink; removed unused icon and styles.
src/components/Pager/Pager.tsx Refactored Pager to a named function; replaced pagination items with DataCiteButton components; simplified URL logic.
src/data/constants.ts Added new color constant ACCENT_COLOR.
src/styles.css Removed old button and pagination styles; added .secondary color class.
src/utils/session.ts Added explicit type assertion to the object returned by session().

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Layout
    participant DiscoverWorksAlert
    participant Session

    Layout->>DiscoverWorksAlert: Render inside layout
    DiscoverWorksAlert->>Session: Get current user session
    alt User is signed in
        DiscoverWorksAlert->>User: Show toast with action buttons
        User->>DiscoverWorksAlert: Click action button (e.g., search works)
        DiscoverWorksAlert->>User: Navigate to selected page
    else User not signed in
        DiscoverWorksAlert-->>Layout: Render nothing
    end
Loading
sequenceDiagram
    participant PageComponent
    participant ExampleTextComponent

    PageComponent->>PageComponent: Check if query is empty
    alt Query is empty
        PageComponent->>ExampleTextComponent: Render example text (specialized component)
    else Query exists
        PageComponent->>PageComponent: Render search results
    end
Loading

Suggested reviewers

  • jrhoads

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fcd6e5 and eeda68c.

📒 Files selected for processing (1)
  • src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: test / cypress (5)
  • GitHub Check: test / cypress (4)
  • GitHub Check: test / cypress (3)
  • GitHub Check: test / cypress (2)
  • GitHub Check: test / cypress (1)
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (18)
src/components/ForceDirectedGraph/ForceDirectedGraph.tsx (1)

46-46: Suppressing TypeScript errors should be justified
The @ts-expect-error directive silences the TypeScript error on the <Vega> component but bypasses compile-­time checks and may mask real issues. Consider importing or declaring the correct prop types for react-vega (e.g. extending the Vega component’s spec and data interfaces) or adding a clear comment referencing the upstream type mismatch for future maintainability.

src/components/SankeyGraph/SankeyGraph.tsx (1)

91-91: Suppressing TypeScript errors should be justified
As in ForceDirectedGraph, the @ts-expect-error directive hides the TS error on the <Vega> component. To maintain type safety, consider defining or importing accurate types for the sankeySpec output and component props, or document the suppression rationale with a reference to a tracking issue.

src/data/constants.ts (1)

46-48: Ensure consistent theming and usage of accent color constant.

The new ACCENT_COLOR constant provides a central definition for the blue accent used across the project—great for maintainability. Please verify:

  • That this constant is used in all relevant places (e.g., the new button and alert SCSS modules) instead of hardcoding #00B1E2.
  • Consider grouping it with other color constants (e.g., in a COLORS object) to align with potential theming strategies.
src/components/FacetList/FacetListGroup.module.scss (1)

2-13: Reevaluate use of !important flags.

Applying !important to all CSS custom properties can lead to maintenance challenges and conflicts later. Consider removing !important for properties that do not collide with other styles or increasing selector specificity instead. For example:

--bs-accordion-border-width: 0 !important;

could be

--bs-accordion-border-width: 0;

and rely on the module-scoped selector.

src/styles.css (1)

645-649: Encapsulate .secondary in a CSS module if possible.

Adding a global .secondary class may lead to style clashes. Since we have scoped styles in DataCiteButton.module.scss, consider moving this into the button component’s module and exposing it as a variant prop, rather than a global class.

cypress/e2e/searchOrganization.test.ts (1)

74-74: Remove unnecessary export from test file.

The export statement is flagged by static analysis and is unnecessary in a test file. Test files typically don't need to export anything.

-export { }
🧰 Tools
🪛 Biome (1.9.4)

[error] 73-74: Do not export from a test file.

(lint/suspicious/noExportsInTest)

cypress/e2e/searchWork.test.ts (1)

159-159: Remove unnecessary export from test file.

This export statement is flagged by static analysis and is unnecessary in a test file. Test files typically don't need to export anything.

-export { }
🧰 Tools
🪛 Biome (1.9.4)

[error] 158-159: Do not export from a test file.

(lint/suspicious/noExportsInTest)

src/components/Header/ServerButtons.tsx (1)

116-117: Improved SignInButton implementation with consistent styling

The refactoring simplifies the SignInButton by leveraging the DataCiteButton component, which provides consistent styling across the application. This change aligns with modern React patterns by composing components rather than using custom CSS.

However, the original implementation likely included a sign-in icon which has been removed. Consider whether this icon should be preserved for visual consistency.

-    <NavLink id="sign-in" href={href} as={DataCiteButton} className="text-nowrap" variant="secondary" outline>
-      Sign In
+    <NavLink id="sign-in" href={href} as={DataCiteButton} className="text-nowrap" variant="secondary" outline icon={faSignInAlt}>
+      Sign In
src/components/DiscoverWorksAlert/DiscoverWorksAlert.module.scss (4)

1-4: Toast container styling looks good, but consider avoiding !important

The toast container styling provides appropriate visual treatment with box-shadow and border, but the use of !important flags suggests potential style conflicts that should be resolved through proper CSS specificity instead.

.toast-container {
-    box-shadow: 0px 28px 36px -10px rgba(13, 96, 212, 0.08) !important;
-    border: 1px solid rgba(36, 59, 84, 0.08) !important;
+    box-shadow: 0px 28px 36px -10px rgba(13, 96, 212, 0.08);
+    border: 1px solid rgba(36, 59, 84, 0.08);
}

6-13: Consider using CSS variables for consistent styling

The primary button styling uses hardcoded color values and multiple !important flags. For better maintainability and consistency, consider using CSS variables for colors and avoiding !important where possible.

+:root {
+    --primary-color: #037AAD;
+    --primary-text-color: #FFF;
+}

.primary {
-    color: #FFF !important;
-    background-color: #037AAD !important;
+    color: var(--primary-text-color);
+    background-color: var(--primary-color);
    border-width: 0 !important;
    font-size: 1em !important;
    padding: .5em 2em !important;
    white-space: nowrap;
}

15-22: Secondary button styling should be consistent with primary

Similar to the primary button, the secondary button uses hardcoded colors and !important flags. For consistency, use the same CSS variables approach recommended for the primary button.

.secondary, .secondary:hover {
-    color: #037AAD !important;
+    color: var(--primary-color);
    background-color: transparent !important;
-    border: 2px solid #037AAD !important;
+    border: 2px solid var(--primary-color);
    font-size: 1em !important;
    padding: .5em 2em !important;
    white-space: nowrap;
}

24-30: Use standard flex-end instead of end for better compatibility

The justify-content: end property might not be widely supported across all browsers. Use flex-end for better compatibility.

.button-container {
    display: flex;
    flex-direction: row;
    flex-wrap: wrap;
-    justify-content: end;
+    justify-content: flex-end;
    gap: 10px;
}
src/components/DataCiteButton/DataCiteButton.module.scss (1)

7-14: Consider removing !important declarations

The stylesheet makes extensive use of !important flags, which can lead to CSS specificity issues and make future styling changes more difficult to implement.

.button {
-    color: $color !important;
-    font-size: 1em !important;
-    padding: .5em 2em !important;
-    border-width: 1.5px !important;
-    border-style: solid !important;
-    border-radius: 100px !important;
+    color: $color;
+    font-size: 1em;
+    padding: .5em 2em;
+    border-width: 1.5px;
+    border-style: solid;
+    border-radius: 100px;
}

If specificity issues arise, consider using more specific selectors or restructuring your CSS hierarchy instead of using !important.

src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx (2)

27-37: Consider adjusting column layout for better centering

The column offset (md={{ span: 8, offset: 3 }}) doesn't properly center the content in a 12-column grid. For proper centering with a span of 8, an offset of 2 would be more appropriate.

-      <Col xs={12} md={{ span: 8, offset: 3 }} className="d-flex justify-content-center">
+      <Col xs={12} md={{ span: 8, offset: 2 }} className="d-flex justify-content-center">

19-19: Consider persisting the alert's dismissed state

Currently, the alert will reappear on page refresh even if the user dismissed it. Consider storing the dismissed state in localStorage or similar to respect the user's preference across sessions.

- const [show, setShow] = useState(true);
+ const [show, setShow] = useState(() => {
+   // Check if running in browser environment
+   if (typeof window !== 'undefined') {
+     const storedValue = localStorage.getItem('discoverWorksAlertDismissed');
+     return storedValue ? false : true;
+   }
+   return true;
+ });

And update the onClose handler:

- <Toast show={show} onClose={() => setShow(false)} className={`${styles["toast-container"]} w-auto my-4 p-2 rounded-0`}>
+ <Toast show={show} onClose={() => {
+   setShow(false);
+   localStorage.setItem('discoverWorksAlertDismissed', 'true');
+ }} className={`${styles["toast-container"]} w-auto my-4 p-2 rounded-0`}>
src/components/DataCiteButton/DataCiteButton.tsx (1)

29-32: Consider simplifying Next.js Link usage.

While the WrapLink helper component works correctly, the use of both passHref and legacyBehavior may be redundant as passHref is implied when using legacyBehavior in Next.js.

-  return <Link href={props.href} passHref legacyBehavior>{props.children}</Link>
+  return <Link href={props.href} legacyBehavior>{props.children}</Link>
src/components/ExampleText/ExampleText.tsx (2)

50-59: Base ExampleText component implemented with good layout structure.

The component uses Bootstrap's grid system appropriately for responsive layout. However, consider improving the documentation link's accessibility.

-  <a href="https://support.datacite.org/docs/datacite-commons" target="_blank" rel="noreferrer">DataCite Support.</a>
+  <a href="https://support.datacite.org/docs/datacite-commons" target="_blank" rel="noreferrer" aria-label="DataCite Support documentation (opens in new tab)">DataCite Support.</a>

62-66: Consider adding accessibility attributes to the Search component icon.

The search icon could benefit from aria attributes to improve accessibility if it's meant to be decorative.

-    <FontAwesomeIcon icon={faSearch} className="me-1" />{props.children}
+    <FontAwesomeIcon icon={faSearch} className="me-1" aria-hidden="true" />{props.children}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f0316c and 8fcd6e5.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (26)
  • cypress/e2e/searchOrganization.test.ts (1 hunks)
  • cypress/e2e/searchPerson.test.ts (0 hunks)
  • cypress/e2e/searchWork.test.ts (1 hunks)
  • src/app/doi.org/page.tsx (3 hunks)
  • src/app/layout.tsx (2 hunks)
  • src/app/orcid.org/page.tsx (1 hunks)
  • src/app/page.tsx (2 hunks)
  • src/app/repositories/page.tsx (2 hunks)
  • src/app/ror.org/page.tsx (2 hunks)
  • src/components/DataCiteButton/DataCiteButton.module.scss (1 hunks)
  • src/components/DataCiteButton/DataCiteButton.tsx (1 hunks)
  • src/components/DiscoverWorksAlert/DiscoverWorksAlert.module.scss (1 hunks)
  • src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx (1 hunks)
  • src/components/DownloadMetadata/DownloadMetadata.tsx (2 hunks)
  • src/components/ExampleText/ExampleText.tsx (1 hunks)
  • src/components/FacetList/FacetListGroup.module.scss (1 hunks)
  • src/components/ForceDirectedGraph/ForceDirectedGraph.tsx (1 hunks)
  • src/components/Header/ClientButtons.tsx (2 hunks)
  • src/components/Header/Dropdown.tsx (1 hunks)
  • src/components/Header/Header.tsx (2 hunks)
  • src/components/Header/ServerButtons.tsx (2 hunks)
  • src/components/Pager/Pager.tsx (2 hunks)
  • src/components/SankeyGraph/SankeyGraph.tsx (4 hunks)
  • src/data/constants.ts (1 hunks)
  • src/styles.css (1 hunks)
  • src/utils/session.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • cypress/e2e/searchPerson.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/app/repositories/page.tsx (1)
src/components/ExampleText/ExampleText.tsx (1)
  • RepositoriesExampleText (39-46)
src/components/DownloadMetadata/DownloadMetadata.tsx (1)
src/components/DataCiteButton/DataCiteButton.tsx (1)
  • DataCiteButton (16-26)
src/app/page.tsx (1)
src/components/ExampleText/ExampleText.tsx (1)
  • WorksExampleText (9-16)
src/components/Header/Header.tsx (1)
src/components/ReactBootstrap.tsx (1)
  • Collapse (17-17)
src/app/doi.org/page.tsx (1)
src/components/ExampleText/ExampleText.tsx (1)
  • WorksExampleText (9-16)
src/app/ror.org/page.tsx (1)
src/components/ExampleText/ExampleText.tsx (1)
  • OrganizationsExampleText (19-26)
src/components/Header/ServerButtons.tsx (2)
src/components/ReactBootstrap.tsx (1)
  • NavLink (14-14)
src/components/DataCiteButton/DataCiteButton.tsx (1)
  • DataCiteButton (16-26)
src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx (3)
src/utils/session.ts (1)
  • session (5-27)
src/data/constants.ts (1)
  • PROFILES_URL (33-33)
src/components/DataCiteButton/DataCiteButton.tsx (1)
  • DataCiteButton (16-26)
src/components/Pager/Pager.tsx (1)
src/components/DataCiteButton/DataCiteButton.tsx (1)
  • DataCiteButton (16-26)
🪛 Biome (1.9.4)
cypress/e2e/searchOrganization.test.ts

[error] 73-74: Do not export from a test file.

(lint/suspicious/noExportsInTest)

cypress/e2e/searchWork.test.ts

[error] 158-159: Do not export from a test file.

(lint/suspicious/noExportsInTest)

⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: test / cypress (5)
  • GitHub Check: test / cypress (4)
  • GitHub Check: test / cypress (3)
  • GitHub Check: test / cypress (2)
  • GitHub Check: test / cypress (1)
  • GitHub Check: CodeQL-Build
🔇 Additional comments (34)
src/components/SankeyGraph/SankeyGraph.tsx (1)

78-79: Formatting adjustments improve readability
The added spaces around the == operator and consistent JSX tag spacing for the <EmptyChart> conditional render align with the repository’s style conventions and enhance readability.

src/styles.css (1)

643-649:

Details

✅ Verification successful

Validate removal of .btn.sign-in and pagination styles.

Global CSS for .btn.sign-in, pagination selectors, and other removed rules are no longer present. Please verify no residual usage remains in the codebase:

This ensures we won’t introduce missing styles or broken UI elements.


🏁 Script executed:

rg --fixed-strings ".btn.sign-in" -n .
rg "pagination a" -n .

Length of output: 63


.btn.sign-in and pagination styles removal verified

  • Searched for .btn.sign-in with rg --fixed-strings ".btn.sign-in" → no matches
  • Searched for pagination a with rg "pagination a" → no matches

No residual references found; it’s safe to remove these styles.

src/app/layout.tsx (1)

59-60: Ensure alert displays only for authenticated users.

The banner should only appear for signed-in users. Verify that DiscoverWorksAlert internally checks the user session and renders null or nothing when no user is present. If not, wrap the component with a conditional:

{session() && <DiscoverWorksAlert />}
src/components/Header/Header.tsx (2)

31-31: Improved spacing between navigation elements.

Adding the gap-5 class to the Collapse component enhances the spacing between child elements, creating a more visually balanced header layout.


45-45: Removed unnecessary top margin from navigation.

Removing the mt-3 class from the Nav component eliminates excess vertical spacing, creating a cleaner alignment with other header elements.

src/components/Header/Dropdown.tsx (2)

9-9: Good defensive programming with explicit error for unauthenticated users.

Adding an explicit error when no user is signed in prevents potential rendering issues and ensures this component is only used in authenticated contexts, aligning with the PR's focus on signed-in user features.


11-11: Consistent vertical spacing for dropdown.

Adding the my-4 class provides consistent margin spacing around the dropdown, improving the visual hierarchy in the header.

src/components/DownloadMetadata/DownloadMetadata.tsx (2)

4-4: Good use of the new DataCiteButton component for consistency.

The replacement of the generic Button with the custom DataCiteButton component contributes to UI consistency across the application. The added props (outline and className='w-100') enhance the button's appearance appropriately.

Also applies to: 15-23


28-35: Successful migration to DataCiteButton in modal footer.

The modal footer button has been properly updated to use the DataCiteButton component with appropriate variant styling.

src/components/Header/ClientButtons.tsx (2)

14-14: Good defensive programming with user session check.

Adding this explicit check for the user's existence prevents potential runtime errors when accessing user.uid if the user is not signed in. The clear error message makes debugging easier.


25-25: Consistent user session validation.

This check mirrors the one added to UserCommonsPageButton, ensuring consistent validation across components that require an authenticated user.

src/app/repositories/page.tsx (1)

3-3: Good refactoring to use specialized example text component.

Replacing inline JSX with the RepositoriesExampleText component improves code organization and maintainability while ensuring UI consistency across search pages. This approach centralizes example text styling and structure, making future updates easier.

Also applies to: 15-16

src/app/orcid.org/page.tsx (1)

3-3: Clean implementation of example text component.

Similar to the repositories page, this refactoring improves code organization by using the specialized PeopleExampleText component. The conditional logic is clear and maintains the same behavior while improving the component structure.

Also applies to: 11-15

src/components/Header/ServerButtons.tsx (1)

5-5: Good addition of the DataCiteButton component

The addition of this import enables the refactoring of the SignInButton to use a more consistent styling approach.

src/app/page.tsx (2)

3-3: Good refactoring of example text import

The import change correctly uses the specialized WorksExampleText component instead of the generic ExampleText.


18-20: Improved code with encapsulated example text component

The replacement of inline JSX with the WorksExampleText component improves code maintainability and consistency across search pages. This change aligns with the broader refactoring effort to standardize example text rendering throughout the application.

src/app/doi.org/page.tsx (2)

5-5: Good addition of specialized example text component

The import of WorksExampleText is appropriate for this DOI search page.


26-28: Improved code organization with standardized example text

The replacement of inline JSX with the WorksExampleText component enhances code consistency and maintainability. This change is consistent with similar refactoring in other search pages, creating a uniform approach to rendering example text.

src/components/DiscoverWorksAlert/DiscoverWorksAlert.module.scss (1)

32-37: Good responsive design consideration

The media query for smaller screens is a good practice to ensure the buttons are usable on mobile devices. Consider aligning the breakpoint with other breakpoints in the application for consistency.

src/app/ror.org/page.tsx (2)

3-3: Nice module refactoring!

The change from importing a generic ExampleText to the specific OrganizationsExampleText component improves code modularity and makes the component's purpose clearer.


18-20: Clean simplification of conditional rendering

Replacing the inline JSX with a dedicated component improves readability and maintainability. This change also ensures consistent user experience across different search pages.

src/components/DataCiteButton/DataCiteButton.module.scss (2)

1-5: Good use of color variables

Defining color variables at the top of the stylesheet promotes consistency and makes future theming changes easier to implement.


41-45: Good responsive design consideration

The media query for small screens ensures buttons are full-width on mobile devices, improving usability on smaller screens.

src/components/DiscoverWorksAlert/DiscoverWorksAlert.tsx (2)

16-22: Good user session check

The component correctly checks for user session before rendering content, aligning with the PR objective to show the alert only to signed-in users.


23-25: Well-constructed URL parameters

The URLs for searching works by ID and name are appropriately constructed, making it easy for users to find their works.

src/components/Pager/Pager.tsx (3)

6-6: Good component refactoring

The refactoring to use the new DataCiteButton component and the change from a const to a named function export improves code consistency and readability.

Also applies to: 15-15


19-24: Cleaner URL construction logic

The simplified logic for determining and constructing page URLs is more readable and maintainable than the previous implementation.


27-33: Improved pagination UI consistency

Using DataCiteButton for pagination controls helps maintain UI consistency across the application. The justify-content-between class appropriately spaces the buttons.

src/components/DataCiteButton/DataCiteButton.tsx (2)

1-14: Well-structured component setup with appropriate imports and props interface.

The component setup follows good practices with the 'use client' directive, necessary imports, and a clear props interface that extends ButtonProps with additional properties for an outline style and icon support.


16-26: Good implementation of conditional styling and icon rendering.

The component correctly applies styling based on variant and outline props, and conditionally renders a FontAwesome icon with appropriate spacing.

src/components/ExampleText/ExampleText.tsx (4)

9-16: Clean implementation of WorksExampleText component.

The component is well-structured with clear URL definitions and appropriate use of the base ExampleText component.


19-26: Good implementation of OrganizationsExampleText component.

Similar to WorksExampleText, this component follows a consistent pattern with clearly defined URLs and appropriate heading.


29-36: Well-structured PeopleExampleText component.

The component follows the established pattern with appropriate example searches for people.


39-46: Clean implementation of RepositoriesExampleText component.

Consistent with the other example text components, providing appropriate repository search examples.

Comment on lines 23 to 27
// verify asymmetric token, using RSA with SHA-256 hash algorithm
JsonWebToken.verify(token, JWT_KEY, { algorithms: ['RS256'] }, setUser)

return user
return user as { uid: string, name: string }
}
Copy link
Contributor

@coderabbitai coderabbitai bot May 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix async JWT verification logic.

JsonWebToken.verify with a callback is asynchronous: the return user as { uid: string, name: string } executes before the callback runs, so user is always null. This breaks session retrieval. Please refactor to use the synchronous return form or Promises:

Example synchronous approach:

export const session = (): { uid: string; name: string } | null => {
  if (!JWT_KEY) return null;
  const sessionCookie = Cookies.getJSON('_datacite');
  const token = sessionCookie?.authenticated?.access_token;
  if (!token) return null;
  try {
    const payload = JsonWebToken.verify(token, JWT_KEY, { algorithms: ['RS256'] }) as any;
    return { uid: payload.uid, name: payload.name };
  } catch (err) {
    console.error('JWT verification error:', err);
    return null;
  }
};
🤖 Prompt for AI Agents
In src/utils/session.ts around lines 23 to 27, the JWT verification uses an
asynchronous callback but immediately returns the user, causing it to always
return null. Refactor the code to use the synchronous form of
JsonWebToken.verify without a callback, wrapping it in a try-catch block to
handle errors. Extract the user info from the returned payload and return it
directly, ensuring the function returns the user only after successful
verification.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bklaing2 Any concerns about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally no, it's been working this far I see no reason to mess with it. I just added the basic typing for clarity when working with the user object

@cypress
Copy link

cypress bot commented May 15, 2025

akita    Run #1673

Run Properties:  status check passed Passed #1673  •  git commit 19f119e993 ℹ️: Merge eeda68caebe32626523c0993e4823694d9e5fb6c into 1f0316c15b36356895c1f1ba26bd...
Project akita
Branch Review discover-works-alert
Run status status check passed Passed #1673
Run duration 01m 37s
Commit git commit 19f119e993 ℹ️: Merge eeda68caebe32626523c0993e4823694d9e5fb6c into 1f0316c15b36356895c1f1ba26bd...
Committer Bryceson Laing
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 3
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 46
View all changes introduced in this branch ↗︎

@bklaing2 bklaing2 merged commit 62fee79 into master May 19, 2025
12 of 13 checks passed
@bklaing2 bklaing2 deleted the discover-works-alert branch May 19, 2025 18:58
@cypress
Copy link

cypress bot commented May 19, 2025

akita    Run #1675

Run Properties:  status check passed Passed #1675  •  git commit 62fee79d4f: Discover works alert (#498)
Project akita
Branch Review 1.37.0
Run status status check passed Passed #1675
Run duration 01m 26s
Commit git commit 62fee79d4f: Discover works alert (#498)
Committer Bryceson Laing
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 3
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 46
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants