add evaluation data page #103

yuki-mt · 2019-05-21T06:30:13Z

What is this PR for?

manage evaluation data from dashboard frontend pages

This PR includes

backend (implementation: 426c176, test: 667cc35 )
- add description to Evaluation model
- add API to download evaluation data file
- add API to get all evaluations
frontend ( a5223ec )
- add link to evaluation page in SideMenu
- show all evaluations as table
- download button to each evaluation row
- upload button for evaluation file
- delete evaluations with checkbox and button

What type of PR is it?

Feature

What is the issue?

N/A

How should this be tested?

python -m unittest test/apis/test_api_evaluation.py

I will add pages for EvaluationResult and EvaluationResult list in the next PRs

codecov-io · 2019-05-21T06:36:44Z

Codecov Report

Merging #103 into master will decrease coverage by 0.48%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   83.73%   83.25%   -0.49%     
==========================================
  Files          43       43              
  Lines        2281     2311      +30     
==========================================
+ Hits         1910     1924      +14     
- Misses        371      387      +16

Impacted Files	Coverage Δ
rekcurd_dashboard/models/evaluation.py	`94.11% <100%> (+0.36%)`	⬆️
rekcurd_dashboard/apis/api_evaluation.py	`41.02% <48.57%> (+0.86%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4508c69...2dc4bd7. Read the comment docs.

test/apis/test_api_evaluation.py

yuki-mt · 2019-05-21T06:38:38Z

rekcurd_dashboard/apis/api_evaluation.py

-            return {"status": True, "evaluation_id": evaluation_model.evaluation_id}
+            raise RekcurdDashboardException(
+                'the file already exists. ID: {}, Description: {}'.format(
+                    evaluation_model.evaluation_id, evaluation_model.description))


I changed return 400 instead of 200 in this case
because it is weird to display as Success but nothing is added to the list page when uploading same content file.

I just noticed that in this way, there is no way to know evaluation_id of existing file contents as JSON response.
So I will add duplicated_ok flag to request

I did it: a631eca

yuki-mt · 2019-05-21T07:04:16Z

related issues: #104 #23

keigohtr

@yuki-mt Thank you for your great work.
Could you fix these comments I made?

keigohtr · 2019-06-15T04:25:14Z

rekcurd_dashboard/apis/api_evaluation.py

-    upload_parser.add_argument('file', location='files', type=FileStorage, required=True)
+    upload_parser.add_argument('filepath', location='files', type=FileStorage, required=True)
+    upload_parser.add_argument('description', location='form', type=str, required=True)
+    upload_parser.add_argument('duplicated_ok', location='form', type=bool, required=False, default=False)


Don't you use duplicated_ok argument from frontend?

@keigohtr
In Frontend, I think the error message is informational enough for duplicated data, so I did not use it

How do we use duplicated_ok argument?

test/apis/test_api_evaluation.py

rekcurd_dashboard/apis/api_evaluation.py

keigohtr · 2019-06-15T04:56:42Z

rekcurd_dashboard/apis/api_evaluation.py

@@ -109,6 +151,29 @@ def delete(self, project_id: int, application_id: str, evaluation_id: int):
        return {"status": True, "message": "Success."}


+@evaluation_api_namespace.route('/projects/<int:project_id>/applications/<application_id>/evaluations/<int:evaluation_id>/download')
+class ApiEvaluationIdDownload(Resource):
+    def get(self, project_id: int, application_id: str, evaluation_id: int):


Could you add a serializer for this endpoint?
It helps user what is the output when we use swagger-ui.

I added in 3fa6c37

I am sorry.
I want you to add @evaluation_api_namespace.marshal_with statement to know the response contents.

frontend/src/components/App/Evaluations/index.tsx

yuki-mt

I fixed some and made some comments.

yuki-mt · 2019-06-29T05:35:19Z

rekcurd_dashboard/apis/api_evaluation.py

-    upload_parser.add_argument('file', location='files', type=FileStorage, required=True)
+    upload_parser.add_argument('filepath', location='files', type=FileStorage, required=True)
+    upload_parser.add_argument('description', location='form', type=str, required=True)
+    upload_parser.add_argument('duplicated_ok', location='form', type=bool, required=False, default=False)


@keigohtr
In Frontend, I think the error message is informational enough for duplicated data, so I did not use it

rekcurd_dashboard/apis/api_evaluation.py

frontend/src/components/App/Evaluations/index.tsx

yuki-mt · 2019-06-29T06:05:46Z

rekcurd_dashboard/apis/api_evaluation.py

@@ -109,6 +151,29 @@ def delete(self, project_id: int, application_id: str, evaluation_id: int):
        return {"status": True, "message": "Success."}


+@evaluation_api_namespace.route('/projects/<int:project_id>/applications/<application_id>/evaluations/<int:evaluation_id>/download')
+class ApiEvaluationIdDownload(Resource):
+    def get(self, project_id: int, application_id: str, evaluation_id: int):


I added in 3fa6c37

keigohtr

@yuki-mt Thank you for fixing!
There is only 1 left we need to fix. Could you check my comment?

keigohtr · 2019-06-30T13:03:27Z

rekcurd_dashboard/apis/api_evaluation.py

+class ApiEvaluationIdDownload(Resource):
+    @evaluation_api_namespace.response(200, description='return evaluation text file.')
+    @evaluation_api_namespace.produces(['text/plain'])
+    def get(self, project_id: int, application_id: str, evaluation_id: int):


Could you add @evaluation_api_namespace.marshal_with here?

This API returns text file, not JSON, so probably there is no way to add marshal_with

(At least, I could not find it..)

I see.
(I also could not find the way...)

rekcurd_dashboard/apis/api_evaluation.py

keigohtr

LGTM
Great work with you!

yuki-mt added 2 commits May 21, 2019 15:13

fix backend for evaluation data

426c176

add test for backend

667cc35

yuki-mt requested a review from keigohtr May 21, 2019 06:30

yuki-mt self-assigned this May 21, 2019

yuki-mt commented May 21, 2019

View reviewed changes

add evaluations page

a5223ec

yuki-mt force-pushed the feature/evaluation-data-frontend branch from 82b97c5 to a5223ec Compare May 21, 2019 07:01

yuki-mt added 4 commits May 22, 2019 11:06

add duplicated_ok flag

a631eca

use default value

bcec000

use state to manage submit button

5a42134

fix bug of notification

ce66b03

This was referenced May 24, 2019

evaluation result list page #105

Merged

update for evaluation rekcurd/airflow-plugin#2

Merged

yuki-mt added 3 commits May 24, 2019 19:38

fix renderFooterButtons

9d10200

fix prevState to nextState

4ec1487

fix prevState to nextState

1018c3a

keigohtr reviewed Jun 15, 2019

View reviewed changes

yuki-mt added 2 commits June 29, 2019 14:04

fix compile error

0d37a38

add decorator for swagger

3fa6c37

yuki-mt commented Jun 29, 2019

View reviewed changes

yuki-mt added 2 commits June 30, 2019 15:17

add auth setting

d963909

remove duplicated_ok

2dc4bd7

yuki-mt force-pushed the feature/evaluation-data-frontend branch from 5cd1bac to 2dc4bd7 Compare June 30, 2019 07:29

keigohtr requested changes Jun 30, 2019

View reviewed changes

keigohtr approved these changes Jun 30, 2019

View reviewed changes

yuki-mt merged commit 616bada into master Jul 6, 2019

yuki-mt deleted the feature/evaluation-data-frontend branch July 6, 2019 10:58

yuki-mt mentioned this pull request Jul 7, 2019

remove duplicated_ok #109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add evaluation data page #103

add evaluation data page #103

yuki-mt commented May 21, 2019 •

edited

Loading

codecov-io commented May 21, 2019 •

edited

Loading

yuki-mt May 21, 2019

yuki-mt May 21, 2019 •

edited

Loading

yuki-mt May 22, 2019

yuki-mt commented May 21, 2019

keigohtr left a comment

keigohtr Jun 15, 2019

yuki-mt Jun 29, 2019

keigohtr Jun 29, 2019

keigohtr Jun 15, 2019

yuki-mt Jun 29, 2019

keigohtr Jun 29, 2019

yuki-mt left a comment

yuki-mt Jun 29, 2019

yuki-mt Jun 29, 2019

keigohtr left a comment

keigohtr Jun 30, 2019

yuki-mt Jun 30, 2019

keigohtr Jun 30, 2019

keigohtr left a comment

add evaluation data page #103

add evaluation data page #103

Conversation

yuki-mt commented May 21, 2019 • edited Loading

What is this PR for?

This PR includes

What type of PR is it?

What is the issue?

How should this be tested?

codecov-io commented May 21, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

yuki-mt May 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki-mt commented May 21, 2019

keigohtr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki-mt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keigohtr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keigohtr left a comment

Choose a reason for hiding this comment

yuki-mt commented May 21, 2019 •

edited

Loading

codecov-io commented May 21, 2019 •

edited

Loading

yuki-mt May 21, 2019 •

edited

Loading