search

erasmus-without-paper / ewp-specs-api-iias-approval

Specifications of EWP's Interinstitutional Agreements Approval API
MIT License
0 stars 0 forks source link

Comments with an explanation why a certain IIA cannot approved #5

Closed jiripetrzelka closed 1 year ago

jiripetrzelka commented 1 year ago

There is currently no way to tell the partner why a certain IIA cannot be approved. Coordinators have to contact the partner by e-mail and explain which IIA they are actually talking about. I think it would be easier if we extended this API and allowed to insert not only approvals but also comments, for example:

<iias-approval-response>

    <approval>
        <iia-id>0f7a5682-faf7-49a7-9cc7-ec486c49a281</iia-id>
        <conditions-hash>a96d6d4254326e5c608ea00b1d2cec77f46e12c2c6f8a4c0b57d05f8c6908c29</conditions-hash>
    </approval>

    <comment>
        <iia-id>82a94c1684ce-947c-7ac9-cd7f-287f065a</iia-id>
        <conditions-hash>3d3310935590256ecff21549b45eb4e1662b0f2a2aafcb7d91ea5f271f4d9e07</conditions-hash>
        <explanation>Please add MA study level to SMS mobility as we agreed by e-mail.</explanation>
    </coment>

</iias-approval-response>
umesh-qs commented 1 year ago

This will probably be a better way to invite comments from the partner on our IIA copy, instead of partner having to mention it on their IIA GET API response. We should extend this API for getting status of IIA from approving partners perspective and not include comments on IIA GET API.

janinamincer-daszkiewicz commented 1 year ago

I see the problem with this proposal. IIA Approval Get is triggered by IIA Approval CNR. Partner B sends IIA Approval CNR to partner A to inform him about waiting IIA Approval. In your scenario this would be like telling the partner:

IIA Approval CNR --> I have IIA Approval for you, take it IIA Approval Get --> In a comment 'I do not like your IIA, make some modifications'

How would the partner differentiate between approval being a real approval and approval asking for further changes?

It reminds me of Omobility LA Update when the comment is used for rejection. But now we are talking about the Approval process which would be used to ask for further negotiations. Wouldn't it be more natural to use IIA Update endpoint for that? Or use status field?

umesh-qs commented 1 year ago

API will be changed from "IIA Approval" to "IIA Status". "IIA Approval CNR" will be called "IIA Status CNR" will be triggered when status of the IIA Changes or when there is a new comment. This way status/comments can be shared even without sharing the IIA.

janinamincer-daszkiewicz commented 1 year ago

Yes, this is an option. Thinking aload. Let's compare:

  1. Status shared with the IIA get, comment shared with IIA update (new).
  2. Status and comment shared with IIA Status API (new). IIA Approval removed.

What are pros and cons?

umesh-qs commented 1 year ago

In either case, let us not link comment with "Approval removed", the mistake that was done with LA update API.

janinamincer-daszkiewicz commented 1 year ago

So how the proposal in (2) looks like? We add IIA Status API but keep IIA Approval?

umesh-qs commented 1 year ago

We keep separate status and comments field in the "IIA Status API"

janinamincer-daszkiewicz commented 1 year ago

We keep separate status and comments field in the "IIA Status API"

That for sure. The question is do we still keep IIA Approval API or do we replace it with IIA Status with status='Approval'?

umesh-qs commented 1 year ago

IIA Status wits status='Approval'. No need of approval API

janinamincer-daszkiewicz commented 1 year ago

This is exactly what I had in mind writing in (2) 'IIA Approval removed'.

umesh-qs commented 1 year ago

sorry. It looked like you meant resetting the IIA Approval.

janinamincer-daszkiewicz commented 1 year ago

I am leaving now (classes start) but if anybody would like to elaborate this idea in more detail fill free to go ;)

umesh-qs commented 1 year ago

I would like to avoid update API(s) as it will be create manual work to go back to the partner if the update fails for some reason. Rather I would keep the control with me and call partner API to get the status even when the CNR fails.

jiripetrzelka commented 1 year ago

I am for the second option, i.e. Status and comment shared with IIA Status API (new).

As for the other issue (https://github.com/erasmus-without-paper/general-issues/issues/38) the proposal was about the ability to comment on own IIAs, not partners' IIAs. So we still need to decide whether we need that. I think that sharing comments regarding own IIAs is less important than the ability to share the reason for not approving partner IIAs but it might still be somewhat useful.

janinamincer-daszkiewicz commented 1 year ago

Let's assume that there is a new IIA Status API (with parameters: 'status of the local copy', 'comment on both copies', 'timestamp') and IIA Status CNR API.

IIA Status API could be used to comment on, let say, status of the negotiations. Partner A sends to partner B: Status='In progress', Comment='You suggest 5 students in ISCED=xyz, but this is not acceptable for me, I suggest 3 students'. Partner B sends to partner A: Status='In progress', Comment='OK, then let's agree on 4 students'.

IIA API would also carry status (of the local copy), but not comment.

This solution would also mean:

  1. withdrawal of the idea of the udpate endpoint to IIA API (suggested in (1) in https://github.com/erasmus-without-paper/ewp-specs-api-iias-approval/issues/3#issuecomment-1114008774);
  2. withdrawal of the IIA Approval and IIA Approval CNR, they would be replaced by IIA Status with status='Approved'? This is more tricky as currently Approval carries also hash and the meaning is 'I approve YOUR copy as identified by this hash'. Should IIA Status API carry statuses of both copies and hash of the partner copy? The meaning is getting more fuzzy. My choice would be: add IIA Status to inform about LOCAL status with a comment, still use IIA Approval to Approve REMOTE copy. Approval has some legal meaning (at least now before electronic signatures are introduced, see https://github.com/erasmus-without-paper/ewp-specs-api-iias/issues/48#issuecomment-1294548770. Also we already have it implemented in our systems. I would still suggest to add the local identifier, as stated in (2) in https://github.com/erasmus-without-paper/ewp-specs-api-iias-approval/issues/3#issuecomment-1287832484.
demilatof commented 1 year ago

I think that @jiripetrzelka described a real problem, but I don't like the idea of modifying the approval response because it would impact with the existing code and with the IIA Approval API specification that says that approval response is for approving agreements

This API allows HEIs to approve agreements sent by their partners https://github.com/erasmus-without-paper/ewp-specs-api-iias-approval

Moreover, it could bring misunderstandings: are you approving or not?

I suggest to manage this problem as addressed by the IIA Approval specification for IDs not approved:

_Values of iia_id that are not known by the server as owner_heiid's agreements or not approved (yet or at all) MUST be ignored. Servers MUST return a valid (HTTP 200) XML response in such cases, but the response will simply not contain any information on these missing entities. I

Therefore, no more an empty response, but a response with a message (that is, the "comment"). If the response is not empty, the IIA ID is known, but we cannot approve it and the message explains why (we need modifications, the hash code is different from the one we considered when we sent the Approval CNR and so on). If the response is empty the IIA ID is unknown.

I think that this solution could be more linear and easy both to be implemented and to be extended for further similar needs.

jiripetrzelka commented 1 year ago

I see several subquestions here:

1. Should comments be published and made available to the counterparty (the way Approval API works) or should they be sent to partner by invoking their API (the way for example LA Update API works)? I think the first option would suffice - i.e. just make the comment available to partner + inform them by a CNR.

2. Should comments be bound to an IIA in general or to a specific conditions-hash? That would depend on the option chosen in the first point. In case of just publishing the comment, you would need the conditions-hash because you don't know when partner gets the comment and the IIA could change in the meantime. Otherwise (if it worked like LA Update API), a timestamp could be enough.

3. Should it be possible to have more than one comment for a certain version of the IIA (= a certain conditions-hash)? I don't think this is necessary if we bind each comment to a certain conditions-hash.

4. Should the comments be part of the existing Approval API? Considering that we would have to at least rename the Approval API to something more generic and increase the major version number, it would be cleaner to have a new API.

5. Should we be able to comment on own IIAs, partner IIAs, or both? In my opinion, both would be ideal. Perhaps the new API could be used for comments in both directions. It would just require some additional parameters. See the example below:

To publish a comment of own IIA, muni.cz would publish for example:

<iias-comment-response>
    <comment>
        <iia-id>1234</iia-id>
        <owner-hei-id>muni.cz<owner-hei-id>
        <commenting-hei-id>muni.cz<commenting-hei-id>
        <explanation>This is the proposal we agreed on by e-mail yesterday.</explanation>
        <conditions-hash>1111</conditions-hash>
        <timestamp>...</timestamp>
    </comment>
<iias-comment-response>

Similarly, muni.cz could comment on partner IIA through the same API:

<iias-comment-response>
    <comment>
        <iia-id>5678</iia-id>
        <owner-hei-id>uw.edu.pl<owner-hei-id>
        <commenting-hei-id>muni.cz<commenting-hei-id>
        <explanation>Please change the number of students from 2 to 3 per year.</explanation>
        <conditions-hash>2222</conditions-hash>
        <timestamp>...</timestamp>
    </comment>
<iias-comment-response>
janinamincer-daszkiewicz commented 1 year ago

I will try to summarise this issue up to now:

  1. @jiripetrzelka , @umesh-qs, @janinamincer-daszkiewicz discuss the new IIA Status and IIA Status CNR APIs, for exchanging information about the current status of the IIA (this is still the open issue if and what status is needed) and some comment on the status of negotiations between partners (details yet to be elaborated).
  2. @demilatof suggests changes in IIA Approval API. Let me explain how I understand this proposal. Any time partner A can request IIA Approval, even if partner B has not yet send IIA Approval CNR. The answer to IIA Approval can be:

Is that correct?

demilatof commented 1 year ago

@janinamincer-daszkiewicz

2. suggests changes in IIA Approval API. Let me explain how I understand this proposal. Any time partner A can request IIA Approval, even if partner B has not yet send IIA Approval CNR. The answer to IIA Approval can be:

Not exactly, in my opinion the IIA Approval CNR is still mandatory, otherwise B could not manage automatically the A Approval request. To clarify my concerns, I think it could happen something after the IIA Approval CNR and before the Approval:

If we don't implement any control, B sends a response to confirm the approval, with the old hash code (X) if it wants to consider only its Approval CNR; otherwise B performs a new IIA-Get and it sends a response to A containing the new hash code (Y) that refers to an IIA that was never seen by IRO. Or B sends an empty response, but A could not understand the reason.

Who should raise this problem? In my opinion B, to avoid giving an approval to the wrong IIA, that was never seen by IRO. Otherwise, B could insist and approve the old copy (old hash code), but afterthat A cannot inform B (via EWP) that its response refers to the old hash code. And therefore B believes to have approved the right IIA.

I think that B should notify A that the IIA is changed (and that B's IRO must examine it again before sending a new IIA Approval CNR); B could perform such a notification by means of the XML Approval response.

* empty - no approval is ready for this IIA.

Not exactly; the answer to Approval will remain empty for unknown IIA Id, as described by the API specification 

* carrying iia-id and conditions-hash - IIA with this hash has been approved.

Yes, this is the response as currently required by the API specification. The only improvement is that should be provided IF and ONLY IF the hash code saved when the Approval CNR was sent is identical to the current hash code (hoping that there are no changes in the last 5 seconds)

* carrying iia-id, conditions-hash and comment - IIA with this hash has not been approved and the comment explains why.

In my implementations we don't need carrying iia-id and conditions-hash, because we manage one IIA per request; but to be more generic, I think that you're right, we need those elements in the response (and we have to include them even if we manage only an IIA per request)

janinamincer-daszkiewicz commented 1 year ago

Not exactly, in my opinion the IIA Approval CNR is still mandatory, otherwise B could not manage automatically the A Approval request.

I am not suggesting that it is not. Of course it is. But anyway, the partner can call get anytime.

jiripetrzelka commented 1 year ago
  1. @jiripetrzelka , @umesh-qs, @janinamincer-daszkiewicz discuss the new IIA Status and IIA Status CNR APIs, for exchanging information about the current status of the IIA (this is still the open issue if and what status is needed) and some comment on the status of negotiations between partners (details yet to be elaborated).

I would prefer to call the new API "Comment API" and design it so that it would serve just for exchanging plaintext comments concerning both own and partner IIAs.

To get the current status of the IIA, you would call the IIA Get API and there you would always have the most up-to-date information about the status. To include status in these comments is in my opinion redundant.

umesh-qs commented 1 year ago
  1. @jiripetrzelka , @umesh-qs, @janinamincer-daszkiewicz discuss the new IIA Status and IIA Status CNR APIs, for exchanging information about the current status of the IIA (this is still the open issue if and what status is needed) and some comment on the status of negotiations between partners (details yet to be elaborated).

I would prefer to call the new API "Comment API" and design it so that it would serve just for exchanging plaintext comments concerning both own and partner IIAs.

To get the current status of the IIA, you would call the IIA Get API and there you would always have the most up-to-date information about the status. To include status in these comments is in my opinion redundant.

It is not redundant. It doesn't makes sense to send a complete agreement that is delete or terminate or sharing is stopped

jiripetrzelka commented 1 year ago

It is not redundant. It doesn't makes sense to send a complete agreement that is delete or terminate or sharing is stopped

In case of deletion, it might make sense but in case of termination, you would still need to obtain the academic year from which the IIA is being terminated from the IIA Get API.

In case of the draft/ready/approved, the state transferred in this API could easily become stale. You would be forced to send a Status/Comment each time the status changes.

The proposition to include states in this new API would make sense to me only in case the state would not be inferrable from the IIA Get itself. In that way, the single source of truth in regards of states would be this new API. But I don't like the idea to move such an important information away from IIA Get. On the other hand, I see that this idea may be attractive in case your goal is to keep the IIA Get API unchanged and reduce the amount of work necessary to implement a new version of the IIA Get API.

umesh-qs commented 1 year ago

It is not redundant. It doesn't makes sense to send a complete agreement that is delete or terminate or sharing is stopped

In case of deletion, it might make sense but in case of termination, you would still need to obtain the academic year from which the IIA is being terminated from the IIA Get API.

In case of the draft/ready/approved, the state transferred in this API could easily become stale. You would be forced to send a Status/Comment each time the status changes.

The proposition to include states in this new API would make sense to me only in case the state would not be inferrable from the IIA Get itself. In that way, the single source of truth in regards of states would be this new API. But I don't like the idea to move such an important information away from IIA Get. On the other hand, I see that this idea may be attractive in case your goal is to keep the IIA Get API unchanged and reduce the amount of work necessary to implement a new version of the IIA Get API.

IIA once shared might be stopped, as I would not like to share the IIA that is undergoing changes. This scenario also needs to be accommodated. 'Draft' might not be appropriate.

janinamincer-daszkiewicz commented 1 year ago

I shared some ideas in https://github.com/erasmus-without-paper/ewp-specs-api-iias/issues/41#issuecomment-1309468065.

jiripetrzelka commented 1 year ago

I shared some ideas in erasmus-without-paper/ewp-specs-api-iias#41 (comment).

On the link above there is no mention of comments regarding partner IIAs. What are the BPOs arguments for discarding this proposal, which would enable HEIs to communicate to the counterparty the reasons why their IIA cannot/will not be approved (yet)?

janinamincer-daszkiewicz commented 1 year ago

On the link above there is no mention of comments regarding partner IIAs. What are the BPOs arguments for discarding this proposal, which would enable HEIs to communicate to the counterparty the reasons why their IIA cannot/will not be approved (yet)?

Why cannot comments regarding partner IIAs be send in the same comment field of IIA? Comment is a text field, it may keep any text, on both copies. Partner B, instead of sending IIA Approval CNR, and then explaining in comment in IIA Approval get the reason of NOT approving, would send IIA CNR, and explanation in the comment in IIA get.

For example: Partner A: changes number of mobilities from 5 to 6. Partner B: don't want to approve such change, so still keeps 5 in his copy, adds to the comment 'I don't agree to rasing the number from 5 to 6' and sends IIA CNR. Partner A: calls IIA get, and in the comment find the explanation.

No approval is involved.

What is wrong in such approach?

demilatof commented 1 year ago

What is wrong in such approach?

In my opinion you're using B's IIA as a communication system, but this is not what it was made for. You approach requires to edit B's IIA when the problem was in A's IIA. And this solution requires more effort from the developers and operators.

Whilst B system could fill automatically an answer to deny the approval (e.g.: hash is changed), if we have to use IIA CNR and a comment in the IIA, we force the system to notify the operator that he/she has to write a comment and send an IIA CNR to A. Yes, we might do this automatically anyway, but this means that the system must substitute itself to the operator to modify the IIA (e.g. adding a comment and then sending a CNR). And in the meantime the operator could have made some changes that he/she wasn't ready yet do notify to the other party.

Therefore, this solution force A to check B's IIA because the comment could be not the only change. And B operator may not be enthusiast that an IIA CNR was sent when he/she wasn't ready.

A lot of additional troubles to solve badly an easy problem.

jiripetrzelka commented 1 year ago

What is wrong in such approach?

You assume that both IIAs are already linked and that both partners have the same link in their respective systems.

janinamincer-daszkiewicz commented 1 year ago

Infrastructure Forum meeting 2022-12-14. New Comment API proposal has been rejected (see https://github.com/erasmus-without-paper/general-issues/issues/38#issuecomment-1356184410).

As agreed during IF, I close old issues and will create a new one, for final discussion and decision on comments in context of IIAs.

janinamincer-daszkiewicz commented 1 year ago

Continued in https://github.com/erasmus-without-paper/ewp-specs-api-iias/issues/97.