[API] PUT and PATCH methods added to Candidates visit/instruments/flags #6780

spell00 · 2020-06-29T20:11:55Z

Brief summary of changes

The methods handlePUT and handlePATCH were added to the endpoint /candidates/<candid>/<visit>/instruments/<instrument>.

Testing instructions

Testing the PUT handle method

Go to https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags in a browser.
The fields Data_entry, Administration and Validity field should be NULL
In a terminal, login by sending the POST request below:
~$ curl https://<hostname>/api/v0.0.3/login -d '{"username": "<username>", "password": "<password>"}'
The expected response is: {"token": ""}. Store the token in a variable token='<a-really-long-string>'.
In the terminal, enter curl -X PUT -H "Authorization: Bearer $token" https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags -d '{"Flags": {"Data_entry": "In Progress", "Administration": "All", "Validity": "Valid"}}'
Go back to https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags in the same browser as in step 1. Data_entry should be "In Progress", Administration should be "all" and Validity should be "Valid".
In the terminal, enter curl -X PUT -H "Authorization: Bearer $token" https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi -d '{"Flags": {"Data_entry": "In Progress", "Administration": null, "Validity": "Questionable"}}'
The field Administration should be changed to null and Validity should be "Questionable"

Testing the PATCH handle method

Go to https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags in a browser.
The fields Data_entry field should be NULL , Administration should be "Partial" and Validity should be "Questionable"
In the terminal, enter curl -X PATCH -H "Authorization: Bearer $token" https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags -d '{"Flags": {"Data_entry": null, "Administration": "All"}}'
Go back to https://<hostname>/api/v0.0.3/candidates/300002/V1/instruments/aosi/flags in the same browser as in step 1. The Data_entry value should now be updated without changing anything else. You should now see "Data_entry" is null ,Validity is still "Questionable" and see that Administration is All in the json string

Link(s) to related issue(s)

Resolves [API] The endpoint /candidates/{candid}/{visit}/instruments/{instrument}/flags needs methods to handle PUT and PATCH requests #6777

xlecours · 2020-07-02T13:01:36Z

This is exactly how it should be done.
I still need to test it though.

Wait, this is for the flags but the code updates the instrument's data.

modules/api/php/endpoints/candidate/visit/instrument/flags.class.inc

spell00 · 2020-08-21T21:57:40Z

I corrected the mistakes in the testing instructions to actually test the PUT and PATCH requests for the flags endpoint instead of instrument , plus it is now actually working

spell00 · 2020-08-24T20:10:33Z

Thanks @xlecours for your suggestion, using NDB_BVL_InstrumentStatus.class.inc makes much more sense! The PUT and PATCH functions work for me now with this branch using the testing instructions in the PR description.

modules/api/php/endpoints/candidate/visit/instrument/flags.class.inc

christinerogers · 2020-08-27T19:51:03Z

@spell00 the ball is in your court on this one i believe.

php/libraries/NDB_BVL_InstrumentStatus.class.inc

modules/api/php/endpoints/candidate/visit/instrument/flags.class.inc

spell00 · 2020-10-29T17:51:27Z

@driusan This PR is approaved and is is ready for your review

…es isntruments flags

Co-authored-by: Xavier Lecours <xavier.lecours@mcin.ca>

spell00 · 2020-11-05T02:56:19Z

@xlecours @driusan It is now passing Travis and ready for final review

driusan · 2020-11-05T14:18:17Z

See my comments on #7088.. I think the lines I commented on there were just in that PR because it builds on this one.

driusan · 2020-11-05T14:18:42Z

modules/api/php/endpoints/candidate/visit/instrument/flags.class.inc

+     */
+    private function _handlePUT(ServerRequestInterface $request) : ResponseInterface
+    {
+        // TODO :: Check permissions. How??


See comment on #7088

are you sure you didn't mean your comment on #7075 ? I applied the change here, as it builds on this one.

data_entry is already checked. The second point is resolved in the parent endpoint modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc (the part commented by Xavier below), so a user without permission can't reach this function.

modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc

xlecours · 2020-11-05T19:45:28Z

modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc

+
+        if ($this->_instrument->_hasAccess($user)) {
+            return new \LORIS\Http\Response\JSON\Forbidden(
+                'Can not update instruments for user' . $user


I don't think $user is a string.

_hasAccess does not take a string

Loris/php/libraries/NDB_BVL_Instrument.class.inc

Line 302 in c1bb1da

function _hasAccess(\User $user) : bool

nevermind I just got it, I'll remove the parameter $user from the message

spell00 · 2020-11-05T23:22:07Z

See my comments on #7088.. I think the lines I commented on there were just in that PR because it builds on this one.

Maybe you mean #7075? it is the PR that builds on this one. I checked the User is permitted to update instruments in modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc line 97, where I use

if ($this->_instrument->_hasAccess($user)) { ...

spell00 · 2020-11-06T16:24:51Z

@xlecours @driusan I did the corrections, this is ready for re-review

driusan

This looks like it's missing a check for the data_entry permission. From what I can tell determineDataEntryAllowed isn't sufficient, it just checks the 'Complete' flag in the backend, not the permissions.

driusan

lgtm.. @xlecours does it still look good to you?

…gs (aces#6780) The methods handlePUT and handlePATCH were added to the endpoint /candidates/<candid>/<visit>/instruments/<instrument>. Resolves aces#6777

spell00 requested a review from xlecours June 30, 2020 19:30

spell00 changed the base branch from master to main July 23, 2020 14:22

spell00 force-pushed the 2020-06-29-AddInstrumentsFlagsPutPatchHandles branch from 9309a91 to 814ec13 Compare August 21, 2020 00:28

spell00 commented Aug 21, 2020

View reviewed changes

modules/api/php/endpoints/candidate/visit/instrument/flags.class.inc Outdated Show resolved Hide resolved

spell00 changed the title ~~[API] PUT and POST methods added to Candidates visit/instruments/flags~~ [API] PUT and PATCH methods added to Candidates visit/instruments/flags Aug 24, 2020

christinerogers requested review from xlecours and removed request for xlecours August 24, 2020 21:19

xlecours requested changes Aug 25, 2020

View reviewed changes

christinerogers assigned spell00 Aug 27, 2020

spell00 force-pushed the 2020-06-29-AddInstrumentsFlagsPutPatchHandles branch from e0fa515 to db5a7c9 Compare September 14, 2020 20:45

spell00 requested a review from xlecours September 17, 2020 22:57

spell00 added the Area: API PR or issue related to the API label Sep 17, 2020

spell00 commented Sep 17, 2020

View reviewed changes

php/libraries/NDB_BVL_InstrumentStatus.class.inc Outdated Show resolved Hide resolved

spell00 commented Sep 17, 2020

View reviewed changes

php/libraries/NDB_BVL_InstrumentStatus.class.inc Outdated Show resolved Hide resolved

spell00 commented Sep 17, 2020

View reviewed changes

php/libraries/NDB_BVL_InstrumentStatus.class.inc Outdated Show resolved Hide resolved

xlecours requested changes Sep 18, 2020

View reviewed changes

xlecours mentioned this pull request Sep 18, 2020

Transition to API version 0.0.4 #6944

Merged

spell00 force-pushed the 2020-06-29-AddInstrumentsFlagsPutPatchHandles branch from 163e8ad to 831e1f1 Compare September 28, 2020 19:46

spell00 mentioned this pull request Oct 22, 2020

Fixes the integration tests of the Candidate Instruments endpoints #7075

Merged

spell00 requested review from driusan and xlecours October 22, 2020 18:33

christinerogers added the Blocking PR should be prioritized because it is blocking the progress of another task label Oct 23, 2020

xlecours approved these changes Oct 26, 2020

View reviewed changes

spell00 mentioned this pull request Oct 26, 2020

[API] New directory for jupyter notebooks #6854

Merged

spell00 force-pushed the 2020-06-29-AddInstrumentsFlagsPutPatchHandles branch from 831e1f1 to 5eb83b5 Compare October 29, 2020 16:02

spell00 and others added 7 commits November 4, 2020 13:40

[API] Remove useless required Meta fields in PUT request for candidat…

ef24d0c

…es isntruments flags

[API] Makes handlePUT and handlePATCH methods descriptions more verbose

d41003b

Fix travis

d5d5882

commit in github did not actually add the line after handlePUT

e217822

remove useles continue 2s

1c20313

Apply Xavier's suggestions from code review

f71a530

Co-authored-by: Xavier Lecours <xavier.lecours@mcin.ca>

detail

863e5d7

spell00 force-pushed the 2020-06-29-AddInstrumentsFlagsPutPatchHandles branch from d179d73 to 863e5d7 Compare November 4, 2020 18:41

spell00 removed the State: Blocked PR or issue awaiting an external event such as the merge or another PR to proceed label Nov 5, 2020

driusan reviewed Nov 5, 2020

View reviewed changes

checking for user permission for instrument

cf4b0e0

spell00 commented Nov 5, 2020

View reviewed changes

modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc Outdated Show resolved Hide resolved

fix travis

9249d1e

xlecours requested changes Nov 5, 2020

View reviewed changes

removing not string parameter from string

c5bed65

spell00 requested a review from xlecours November 5, 2020 20:10

spell00 requested a review from driusan November 6, 2020 06:33

spell00 added 2 commits November 6, 2020 02:22

remove detail

6f9b45a

final

7c69af2

driusan requested changes Nov 6, 2020

View reviewed changes

Check for data_entry

9c12b7c

driusan approved these changes Nov 6, 2020

View reviewed changes

xlecours approved these changes Nov 9, 2020

View reviewed changes

driusan merged commit 920fd4e into aces:main Nov 9, 2020

ridz1208 added this to the 24.0.0 milestone Nov 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[API] PUT and PATCH methods added to Candidates visit/instruments/flags #6780

[API] PUT and PATCH methods added to Candidates visit/instruments/flags #6780

spell00 commented Jun 29, 2020 •

edited

Loading

xlecours commented Jul 2, 2020 •

edited

Loading

spell00 commented Aug 21, 2020 •

edited

Loading

spell00 commented Aug 24, 2020

christinerogers commented Aug 27, 2020

spell00 commented Oct 29, 2020

spell00 commented Nov 5, 2020

driusan commented Nov 5, 2020

driusan Nov 5, 2020

spell00 Nov 5, 2020

spell00 Nov 5, 2020 •

edited

Loading

xlecours Nov 5, 2020

spell00 Nov 5, 2020

spell00 Nov 5, 2020 •

edited

Loading

spell00 Nov 5, 2020

spell00 commented Nov 5, 2020 •

edited

Loading

spell00 commented Nov 6, 2020

driusan left a comment

driusan left a comment

[API] PUT and PATCH methods added to Candidates visit/instruments/flags #6780

[API] PUT and PATCH methods added to Candidates visit/instruments/flags #6780

Conversation

spell00 commented Jun 29, 2020 • edited Loading

Brief summary of changes

Testing instructions

Testing the PUT handle method

Testing the PATCH handle method

Link(s) to related issue(s)

xlecours commented Jul 2, 2020 • edited Loading

spell00 commented Aug 21, 2020 • edited Loading

spell00 commented Aug 24, 2020

christinerogers commented Aug 27, 2020

spell00 commented Oct 29, 2020

spell00 commented Nov 5, 2020

driusan commented Nov 5, 2020

driusan Nov 5, 2020

Choose a reason for hiding this comment

spell00 Nov 5, 2020

Choose a reason for hiding this comment

spell00 Nov 5, 2020 • edited Loading

Choose a reason for hiding this comment

xlecours Nov 5, 2020

Choose a reason for hiding this comment

spell00 Nov 5, 2020

Choose a reason for hiding this comment

spell00 Nov 5, 2020 • edited Loading

Choose a reason for hiding this comment

spell00 Nov 5, 2020

Choose a reason for hiding this comment

spell00 commented Nov 5, 2020 • edited Loading

spell00 commented Nov 6, 2020

driusan left a comment

Choose a reason for hiding this comment

driusan left a comment

Choose a reason for hiding this comment

spell00 commented Jun 29, 2020 •

edited

Loading

xlecours commented Jul 2, 2020 •

edited

Loading

spell00 commented Aug 21, 2020 •

edited

Loading

spell00 Nov 5, 2020 •

edited

Loading

spell00 Nov 5, 2020 •

edited

Loading

spell00 commented Nov 5, 2020 •

edited

Loading