tackle-75-business-logic-for-create-assessment

[jonathanvila]

Issue : #11

Features included :

• POST assessments operation now will copy the default (first one) Questionnaire structure ( categories, questions, options ) into the assessment
• Old Pathfinder questionnaire inserted as the default questionnaire

Pre steps :

1. Deploy tackle-controls ( to have the keycloak instance running ) on your K8s cluster

$ kubectl apply -f { tackle-controls kubernetes deployment file }

2. Edit the Ingress object to allow direct request to Pathfinder API

$ kubectl edit ingress tackle -n tackle

Add this below paths:

     - backend:
          service:
            name: tackle-pathfinder
            port:
              number: 8080 
        path: /pathfinder
        pathType: ImplementationSpecific

3. Build and push

$ ./mvnw -U -B package -Dquarkus.container-image.push=true -Dquarkus.container-image.group={your quay user} -Dquarkus.container-image.registry=quay.io -Dquarkus.container-image.username={your quay user} -Dquarkus.container-image.password={your quay pwd} -Pnative

4. Change the image in the deployment file (src/main/kubernetes/tackle-pathfinder.yaml) line 224 to be the one you have pushed
5. Deploy tackle-pathfinder on your K8s cluster , same namespace as tackle-controls

$ kubectl apply -f src/main/kubernetes/tackle-pathfinder.yaml 

Test case 1 : Obtain Keycloak secret

$ export access_token=$(\
    curl -X POST http://{cluster IP}/auth/realms/quarkus/protocol/openid-connect/token \
    --user backend-service:secret \
    -H 'content-type: application/x-www-form-urlencoded' \
    -d 'username=alice&password=alice&grant_type=password' | jq --raw-output '.access_token' \
 )

Expectation test case 1

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1825  100  1776  100    49  18500    510 --:--:-- --:--:-- --:--:-- 19010

Test case 2 : Create assessment for applicationId=20

curl 'http://{cluster IP}/pathfinder/assessments' \
  -H 'Content-Type: application/json' -H 'Accept: application/json' -H "Authorization: Bearer "$access_token \
  -d "{ \"applicationId\": 20 }" \
  -s -i

Expectation Test case 2

HTTP/1.1 201 Created
Date: Mon, 22 Mar 2021 11:05:29 GMT
Content-Type: application/json
Content-Length: 46
Connection: keep-alive

{"id":3,"applicationId":28,"status":"STARTED"}

Test case 3 : DB check for assessment for applicationId=20

1. create a postgre client pod with

$ kubectl run pgcli --image riuvshin/postgres-client

2. check status of the pod, and leave ( ^c ) when Ready

$ kubectl get pod pgcli -w 

3. sh into the pod

$ kubectl exec -ti pgcli sh

4. connect to the DB

$ psql --host=pathfinder-postgresql --port=5432 --username=pathfinder --dbname=pathfinder_db

5. execute query

$ select count(*) from assessment_singleoption;

Expectation Test case 3

pathfinder_db=# select count(*) from single_option ;
 count 
-------
   174
(1 row)

Test case 4 : Simultaneous calls to create assessment return 201 for the first and 400 for the rest ( to avoid multiple assessments per application )

1. create a file with this content

export access_token=$(\
    curl -X POST http://$2/auth/realms/quarkus/protocol/openid-connect/token \
    --user backend-service:secret \
    -H 'content-type: application/x-www-form-urlencoded' \
    -d 'username=alice&password=alice&grant_type=password' | jq --raw-output '.access_token' \
 )
time
curl 'http://$2/pathfinder/assessments' \
  -H 'Content-Type: application/json' -H 'Accept: application/json' -H "Authorization: Bearer "$access_token \
  -d "{ \"applicationId\": $1 }" \
  -v -s &
time
curl 'http://$2/pathfinder/assessments' \
  -H 'Content-Type: application/json' -H 'Accept: application/json' -H "Authorization: Bearer "$access_token \
  -d "{ \"applicationId\": $1 }" \
  -v -s &
time
curl 'http://$2/pathfinder/assessments' \
  -H 'Content-Type: application/json' -H 'Accept: application/json' -H "Authorization: Bearer "$access_token \
  -d "{ \"applicationId\": $1 }" \
  -v -s &

2. execute it with bash -x file.sh {incremental number} {cluster ip}
   If you are using minikube you can get your cluster ip with : minikube ip

Expectation Test case 4
Something similar to this ( you have 1 HTTP/1.1 201 , and 2 HTTP/1.1 400 )

< HTTP/1.1 201 Created
< Date: Tue, 13 Apr 2021 08:17:53 GMT
< Content-Type: application/json
< Content-Length: 50
< Connection: keep-alive
< 
* Connection #0 to host 192.168.49.2 left intact
{"id":1051,"applicationId":802,"status":"STARTED"}* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< Date: Tue, 13 Apr 2021 08:17:53 GMT
< Content-Length: 0
< Connection: keep-alive
< 
* Connection #0 to host 192.168.49.2 left intact
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Bad Request
< Date: Tue, 13 Apr 2021 08:17:53 GMT
< Content-Length: 0
< Connection: keep-alive
< 
* Connection #0 to host 192.168.49.2 left intact

[codecov]

Codecov Report

Merging #18 (c231b28) into main (03524b6) will decrease coverage by 13.33%.
The diff coverage is 50.00%.

❗ Current head c231b28 differs from pull request most recent head c8458bc. Consider uploading reports for the commit c8458bc to get more accurate results

@@              Coverage Diff              @@
##               main      #18       +/-   ##
=============================================
- Coverage     91.11%   77.77%   -13.34%     
- Complexity       25       33        +8     
=============================================
  Files             7       16        +9     
  Lines            45       63       +18     
  Branches          1        1               
=============================================
+ Hits             41       49        +8     
- Misses            3       13       +10     
  Partials          1        1               

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/io/tackle/pathfinder/model/Risk.java	0.00% <0.00%> (ø)	0.00 <0.00> (?)
...tackle/pathfinder/model/assessment/Assessment.java	50.00% <ø> (ø)	1.00 <0.00> (ø)
...athfinder/model/assessment/AssessmentCategory.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...athfinder/model/assessment/AssessmentQuestion.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...inder/model/assessment/AssessmentSingleOption.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...ackle/pathfinder/model/questionnaire/Category.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...ackle/pathfinder/model/questionnaire/Question.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...e/pathfinder/model/questionnaire/SingleOption.java	50.00% <50.00%> (ø)	1.00 <1.00> (?)
...nder/model/assessment/AssessmentQuestionnaire.java	66.66% <66.66%> (ø)	1.00 <1.00> (?)
.../pathfinder/model/questionnaire/Questionnaire.java	66.66% <66.66%> (ø)	1.00 <1.00> (?)
... and 10 more

---

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 03524b6...c8458bc. Read the comment docs.

[carlosthe19916]

@jonathanvila I added some comments below and besides my comments I'd like to also mention:

• I've seen the field notes in the Assessment entity; comment in AssessmentQuestion but the comments/notes should be located in the AssessmentCategory. Could we have a single term for both comment and notes? unless they both mean different things.
• Before this PR the creation of an assessment was very quick but with this PR the creation of an assessment takes longer (I guess it is because it needs to copy the questionnaire). Since it takes longer for the backend to process the POST /assessment request I was able to discover that we can create multiple assessments for a single application which I think is breaking the constraint of having a single assessment per application. See the video below (note that in my video I'm using the UI but you can use curl to have the same results). In the video I'm clicking multiple times the "assess" button.

Peek.2021-04-09.11-29.mp4

Of course, the UI should also disable the button "assess" while executing POST /assessment so this is something to enhance also in the UI.

• Could you please format to the .SQL files? I guess it is a matter of using the IDE format feature.

[jonathanvila]

@jonathanvila I added some comments below and besides my comments I'd like to also mention:

• I've seen the field notes in the Assessment entity; comment in AssessmentQuestion but the comments/notes should be located in the AssessmentCategory. Could we have a single term for both comment and notes? unless they both mean different things.

Well, a comment and notes are not exactly the same and that was my intention. But I see your point, and as it's a matter of perception , I will rename to Comment everywhere as you point.

[jonathanvila]

• Before this PR the creation of an assessment was very quick but with this PR the creation of an assessment takes longer (I guess it is because it needs to copy the questionnaire). Since it takes longer for the backend to process the POST /assessment request I was able to discover that we can create multiple assessments for a single application which I think is breaking the constraint of having a single assessment per application. See the video below (note that in my video I'm using the UI but you can use curl to have the same results). In the video I'm clicking multiple times the "assess" button.
  Peek.2021-04-09.11-29.mp4
  Of course, the UI should also disable the button "assess" while executing POST /assessment so this is something to enhance also in the UI.

👍 Good spot .

[carlosthe19916]

@jonathanvila I added just an additional comment to this PR, feel free to take any action about it.

I've tested the latest commit available and the problem of having multiple assessments for an application seems to be solved. I'm sharing the video below of one test I've done using JMeter. This is what I understand from the current status of this PR:

• The problem of multiple assessments per application was solved by adding a UNIQUE constraint to the table assessment. In this way, we are sure that even if the backend tries to create multiple assessments per application the DB won't allow it to happen.
• The line assessment.persistAndFlush() located at https://github.com/jonathanvila/tackle-pathfinder/blob/tackle-75-business-logic-for-create-assessment/src/main/java/io/tackle/pathfinder/services/AssessmentSvc.java#L47 helps to early reject the creation of a new assessment. I've started pathfinder using quarkus:dev and with assessment.persistAndFlush() it takes around 4 seconds for an assessment to be created; on the other hand, using assessment.persist() takes around 10 seconds. The timing might change on a native container.

Peek.2021-04-13.11-53.mp4

[carlosthe19916]

Thanks @jonathanvila , this PR looks good to me.