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
Planner 1580 add frontend service #4
Conversation
Sorry for holding the review off; I might be able to get to it tomorrow or early next week. |
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.
Quickly went though the PR, looks good. The only concern I have are missing tests of the UI, but my understanding is that this is just an example on how to do the task assignment.
Is there any plan regarding where this example should end up? Is it going to be another optaweb project?
spring.jpa.generate-ddl=true | ||
spring.jpa.hibernate.ddl-auto=none | ||
spring.jpa.properties.hibernate.jdbc.lob.non_contextual_creation=true | ||
|
||
logging.level.org.springframework.web=DEBUG | ||
logging.level.org.optaplanner.core=INFO |
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.
Do we want to have both optaplanner and drools on INFO level by default? WARN might be sufficient.
--> | ||
|
||
<solver> | ||
<!--<environmentMode>FULL_ASSERT</environmentMode>--><!-- To slowly prove there are no bugs in this code --> |
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.
Do we want to keep the commented out code for a potential user to play with it or should we rather remove it?
src/main/resources/logback.xml
Outdated
<pattern>%d{HH:mm:ss.SSS} [%-12.12t] %-5p %m%n</pattern> | ||
</encoder> | ||
</appender> | ||
<!--<appender name="fileAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">--> |
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.
Could we remove the unused appender?
import constants from '../shared/constants'; | ||
|
||
// idToColor from "Joe Freeman" answer: https://stackoverflow.com/questions/3426404/create-a-hexadecimal-colour-based-on-a-string-with-javascript | ||
const idToColor = (str) => { |
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.
Soft suggestion: how about moving this special function into a separate .js file?
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.
What's the motivation of moving it? I would've done so if it's to be used in another .js file or it's too long, but in this case it's a short function that's only relevant to ScheduleComponent
, unless you have a stronger argument for moving it.
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.
Just not to spoil the logic of the component with this special detail. As it's not reused from anywhere else, it's not important.
@@ -0,0 +1,47 @@ | |||
## OptaTask UI: | |||
In order to test the functionality, first have a KIE server running on http://localhost:8080 as explained here: https://docs.optaplanner.org/7.12.0.Final/optaplanner-wb-es-docs/html_single/#_planner.quickstart |
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.
Let's refer to some newer version, 7.12.0.Final is quite old.
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.
Actually this instruction is relevant to kie-server-task-assignment
demo, I should remove it altogether.
|
||
In the project directory, you can run: | ||
|
||
### `npm start` |
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.
If we want to help users who are not familiar with npm, let's mention also npm install
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.
This readme is auto generated by create-react-app, I will put the instructions for unfamiliar useres in the parent directory README,
@rsynek thanks for the review! There are no plans currently for an optaweb-task-assignment, and the UI is just for testing and experimental purposes for using SolverManager, that's why there are no tests. |
Add front end with the ability to:
To run the ui:
--
./mvnw spring-boot:run
--
cd spring-boot-task-assigning-frontend
--
npm start