search

erasmus-without-paper / ewp-specs-api-omobility-las

Learning Agreements
MIT License
1 stars 2 forks source link

Does LA API provide reject functionality? #36

Closed umesh-qs closed 1 year ago

kamil-olszewski-uw commented 2 years ago

We are not working on the LA removal API at the moment. In erasmus-without-paper/ewp-specs-api-iias#41, we are discussing a potential IIA removal API. Conclusions and decisions made in the context of IIAs will also be used for LAs.

umesh-qs commented 2 years ago

@kamil-olszewski-uw does it mean that there is no reject option in LA API/Process

kamil-olszewski-uw commented 2 years ago

I'm sorry, I understood from your question that you were asking for LAs removal.

Rejection of the proposed LA version (first or later one) is possible in the update endpoint request:

https://github.com/erasmus-without-paper/ewp-specs-api-omobility-las/blob/0d1044c0cf3be210e30737daedd8de4fd5ad821b/endpoints/update-request.xsd#L109

umesh-qs commented 2 years ago

@kamil-olszewski-uw this is for sending comments. I do not see reject option. Can you please help me understand?

kamil-olszewski-uw commented 2 years ago

Such comment means that you do not accept LA version sent by sending HEI. As the comment in the specification says: "Field to describe why LA has not been accepted and what are the suggested changes."

umesh-qs commented 2 years ago

So any comment irrespective of its purpose will be treated as rejection of LA?

kamil-olszewski-uw commented 2 years ago

Comment means rejection of proposed version of LA. Receiving HEI can't reject whole LA. If mobility does take place then LA should be established.

And if for some reason LA was sent for a non-existent mobility, the comment may contain information that such mobility does not exist and sending HEI should not send other proposals of this LA, if the matter is not cleared.

kkaraogl commented 2 years ago

@umesh-qs haven't we tested this whole flow already based on the scenarios published here https://github.com/erasmus-without-paper/ewp-specs-api-omobility-las/tree/stable-v1/examples/ola-dashboard-example-scenarios?

There is a rejection in the scenarios, the way Kamil describes it.

umesh-qs commented 2 years ago

@kamil-olszewski-uw Is this by design or a afterthought hack?

kamil-olszewski-uw commented 2 years ago

That was how it was decided from the beginning. Do you see any cons of this solution?

umesh-qs commented 2 years ago

@kamil-olszewski-uw .. can you please share some issue link for discussion on this. May be we missed it. Should I assume that the API was designed this way for rejection scenario?

kkaraogl commented 2 years ago

@umesh-qs we've tested this whole flow together, and we are exchanging this way in production with you for months now! Maybe you are asking something else and I got lost.

umesh-qs commented 2 years ago

@kkaraogl .. MoveOn has never treated comments as rejection. If you want we can discuss this one to one.

kkaraogl commented 2 years ago

@umesh-qs but it is described in the scenarios we went through, during testing, as rejection.

Feel free to reach out, of course, and we'll discuss this.

umesh-qs commented 2 years ago

@kkaraogl have sent you and email regarding this and the scenarios that were agreed upon.

janinamincer-daszkiewicz commented 2 years ago

https://github.com/erasmus-without-paper/ewp-specs-api-omobility-las/blob/0d1044c0cf3be210e30737daedd8de4fd5ad821b/endpoints/update-request.xsd#L134 Field to describe why LA has not been accepted and what are the suggested changes.

https://github.com/erasmus-without-paper/ewp-specs-mobility-flowcharts#approving-learning-agreements obraz

Is this by design or a afterthought hack?

By design:

  1. When we approve comment is not needed.
  2. When we reject, we should explain why, so comment is needed.
umesh-qs commented 2 years ago

Not sure if it makes sense to go over the entire approval process (involving all 3 parties) for minor clarifications. It would have been better if comments were served only as means of clarification and there was a separate option for rejection.

Also if comments are meant to be used a rejection then the element should have been named as

janinamincer-daszkiewicz commented 2 years ago

Comment is not for minor clarifications. If anything changes in the LA it has to be approved again by all three parties. For a student every small change makes a difference. Concerning name change I can only say that naming if one of the biggest challenges in programming. There are usually as many proposals as persons involved.

umesh-qs commented 2 years ago

Comment is not for minor clarifications. If anything changes in the LA it has to be approved again by all three parties. For a student every small change makes a difference. Concerning name change I can only say that naming if one of the biggest challenges in programming. There are usually as many proposals as persons involved.

Yes in current design, it is the case. A better design could have been separating comments with or without rejection.

Regarding the element name, had it been thought earlier (not asking to change now), it is more about making it clear in the element name, that the purpose of this is only for rejection. Sometimes it doesn't harm to think beyond the complexity of implementation. Documentation also matters.

janinamincer-daszkiewicz commented 2 years ago

Documentation also matters.

In documentation the meaning is clearly explained.

A better design could have been separating comments with or without rejection.

Our opinion was different, so the design is different.

umesh-qs commented 2 years ago

Oh yes. All the APIs including IIA has everything clear in the documentation and there is no scope of misinterpretation. Guess this is the right approach to follow.

When you say "Our opinion" who are you referring to?