Skip to content

Conversation

@Sameera-05
Copy link

@Sameera-05 Sameera-05 commented Jun 17, 2025

Overview:

This PR adds three new components to the dashboard: Quick Links, Recent Jobs, and System Status. These components aim to enhance the user experience by providing faster access to commonly used features, job summaries, and system health status.

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

  • Added Quicklinks sidebar component to dashboard layout.
  • Implemented JobStatus component to display user's recent job runs.
  • Integrated System Status table using live data from TACC API (via CORS proxy).

Testing Steps:

  1. Navigate to the Dashboard page.

  2. Verify the following:

    - Sidebar with Quicklinks appears on the left.
    - Recent Jobs list is displayed under the dashboard heading.
    - System Status table appears on the right with status, load, and job metrics.
    
  3. Confirm live data is rendered via proxy without CORS issues.

  4. Confirm layout does not break and displays without horizontal scrollbars.

UI Photos:

image

Notes:

  • Currently using https://corsproxy.io to bypass CORS for live system status. Ideally, this should be replaced with a backend proxy for production.

@Sameera-05 Sameera-05 requested a review from jarosenb June 18, 2025 18:49
@@ -0,0 +1,54 @@
import { useState, useEffect } from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

We can use the existing useSystemOverview hook for system status, which proxies tup-services on the backend without having to use a CORS proxy.

Copy link
Author

Choose a reason for hiding this comment

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

Made suggested changes. Used existing useSystemOverview hook. Deleted useGetLiveSystemStatus.ts file.


logger = logging.getLogger(__name__)
def get_detailed_tas_allocations(username):
import logging
Copy link
Member

Choose a reason for hiding this comment

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

The logger is configured at the top of the file, so you don't need to import/define it inside this method.

Copy link
Author

Choose a reason for hiding this comment

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

Removed import logging statement inside get_detailed_tas_allocations(username):

@@ -0,0 +1,32 @@
import { vi } from 'vitest';
Copy link
Member

Choose a reason for hiding this comment

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

Is this setupTests code required to get the initial unit test to pass? If it is, I think it might be masking a deeper issue where we need to be mocking the backend API calls. We haven't really discussed how to do that, and that's an oversight on my part- we can schedule a pair programming session to write some initial tests using mock API calls.

Copy link
Author

Choose a reason for hiding this comment

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

Yes Jake, this setup was mainly added to get the tests passing initially due to issues with matchMedia and IntersectionObserver. But I agree it’s not the ideal long-term solution. Mocking the backend API would definitely be the better approach. I'd appreciate a pair programming session to get started with that!

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, Jake — you were absolutely right. I removed the setupTests.ts file and updated Dashboard.spec.tsx to locally mock matchMedia and getComputedStyle, which were the main blockers. The test now runs successfully without relying on global setup. Let me know if this approach looks good to you — and if not, I’m happy to arrange a pair programming session to improve it further.

'data.tacc.utexas.edu': 'Corral (Storage)',
};

const SUAllocationsCard = () => {
Copy link
Member

Choose a reason for hiding this comment

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

For users that have a lot of allocations, this section takes up a lot of vertical screen space. I think you have some options for how to deal with this:

  • Use the Collapse component so that the projects for each system are hidden until the user clicks on them
  • Add an internal scroll so that the card has a fixed height of around 500px

Copy link
Author

Choose a reason for hiding this comment

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

I had already added the collapsible sections earlier, but I’ve now updated them to stay hidden by default until the user clicks to expand. This helps reduce the vertical space on initial load.
image

const [showJobs, setShowJobs] = useState(true);
const [showAllocations, setShowAllocations] = useState(true);

const columns = [
Copy link
Member

Choose a reason for hiding this comment

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

If a system is down, some of the column cells show invalid values:
image
Let's display (N/A) instead of a value for the Load/Running Jobs/Waiting jobs.

Copy link
Author

@Sameera-05 Sameera-05 Jul 16, 2025

Choose a reason for hiding this comment

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

I’ve updated the table to show (N/A) instead of invalid values for load, running jobs, and waiting jobs when a system is down.

@Sameera-05 Sameera-05 requested a review from fnets July 22, 2025 16:09
Copy link
Contributor

@fnets fnets left a comment

Choose a reason for hiding this comment

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

A couple of small changes, but great work!

Copy link
Contributor

@fnets fnets left a comment

Choose a reason for hiding this comment

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

This is looking really good! I think Jake's requests still need to be addressed, especially the API testing. Once those are done, I think we should be good to merge this. Great job!

@jarosenb
Copy link
Member

jarosenb commented Aug 1, 2025

Closing this PR- dashboard changes have been consolidated into #1612

@jarosenb jarosenb closed this Aug 1, 2025
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