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

Adding Job Creation view, Job Details, refresh of the list, the choice of Collab #23

Merged
merged 43 commits into from
Mar 24, 2021

Conversation

msenoville
Copy link
Collaborator

@msenoville msenoville commented Mar 12, 2021

This is the current status of our contributions. Thanks to @Haguili for the improved JobList. Since the last merge, jobs with 'submitted' and 'running' status have been added to the list. I added a small modification to avoid the 2-step refresh (because of 2 end-points), and to avoid multiple entries for the same job because of the concatenation of 2 lists.

Summary:
This pull request resolves issues #3, #4, #6, #7 and #9, i.e. the implementation of the Job Creation view, the Job Details, the refresh of the list, the choice of Collab, all using Material-UI.

…oncat() is used when the list is reloaded (this avoids multiple entries for the same job.
…ation, since the joblist is sourced from 2 end-points (/results/ and /queue/)
Copy link
Member

@apdavison apdavison left a comment

Choose a reason for hiding this comment

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

Overall this looks great. I am going to merge this now, I will create a new ticket to collect the minor issues I noted.

@@ -3,17 +3,31 @@
"version": "2.0.0dev",
"private": true,
"dependencies": {
"@fortawesome/fontawesome-svg-core": "^1.2.34",
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we use icons only from Material UI, it will be confusing if we mix icons from two different sources

@@ -3,11 +3,61 @@ import axios from 'axios';
import {
useParams
} from "react-router-dom";
//import { Button } from '@material-ui/core';
Copy link
Member

Choose a reason for hiding this comment

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

todo: clean up commented-out code

// import { Prism as SyntaxHighlighter } from 'react-syntax-highlighter';
// import { dark } from 'react-syntax-highlighter/dist/esm/styles/prism';
// <link rel="stylesheet" href="https://highlightjs.org/static/demo/styles/railscasts.css" />
import ExpansionPanel from '@material-ui/core/ExpansionPanel';
Copy link
Member

Choose a reason for hiding this comment

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

I think ExpansionPanel is now called Accordion

@@ -16,39 +66,149 @@ function JobDetail(props) {
}
}
// const resultUrl = `https://raw.githubusercontent.com/jonathanduperrier/nmpi-job-manager-app-reactjs/master/db_${id}.json`;
const resultUrl = `https://nmpi.hbpneuromorphic.eu/api/v2/results/${id}/?collab_id=neuromorphic-testing-private`;
const resultUrl = `https://nmpi.hbpneuromorphic.eu/api/v2/results/${id}`;
Copy link
Member

Choose a reason for hiding this comment

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

todo: move API base URL (https://nmpi.hbpneuromorphic.eu/api/v2/) to a globals.js file



let currentdate = new Date();
let fetchDataDate = "Last updated: " + currentdate.getDate() + "/"
Copy link
Member

Choose a reason for hiding this comment

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

I think we do date formatting in a couple of places, so I suggest we make this a function and move it to a utils.js file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants