Include recommended user messages

[Majsvaffla]

The RFA for the CertificateError exception was changed some version ago (https://github.com/hbldh/pybankid/blob/master/bankid/exceptions.py#L197). The comment was updated but the actual value on the class was not. It should probably be the same as the comment. However, the current RP guidelines says that RFA16 should be used for certificateErr.

This got me thinking; wouldn't it be neat if pybankid included the RFAs?

[hbldh]

It would be nice, yes. It is not something I have time to address though. Feel free to do the change and make a PR!

[Majsvaffla]

I've given this some thought and have come up with some ideas and conclusions.

I guess the easiest solution for this would be to simply include the messages as constants and let the calling code use the proper message based on the API response. However it would be cool if pybankid already had determined the current RFA based on the response.

For the errors that pybankid raises it would be easy to also provide some dictionary to look up the actual messages or even include the messages as attributes on the exception classes themselves. As for the messages that should be used for pending, and canceled orders etc. (where pybankid doesn't raise an exception) the messages (or the number for the message) would have to be returned from jsonclient.sign/collect etc. along with the response data from the BankID API. That would introduce a breaking change however as it would require either returning a tuple or preferably some data class.

I would gladly implement this but I would like your input on which solution you think is the better.

Maybe we could do this in two parts where we include the messages as constants as the first part and then add some resolution logic later on.

[hbldh]

My original plan was to include the rfa id with the exceptions and let the user fetch the corresponding text when dealing with the error. Similarily, I wanted to return the actual BankID JSON data as untouched as possible, and not introduce anything of my own in the data flow that was documented in the BankID specs. Not sure if that is relevant though. It might be

For exceptions I agree that the text can be included, or preferrably fetched from a dictionary of rfa texts.

For regular return data, I would like to not return a tuple, but rather add a field to the output json, e.g. :

{
    "orderRef":"131daac9-16c6-4618-beb0-365768f37288",
    "status":"pending",
    "hintCode":"userSign",
    "rfa": {
        "id": 9,
        "sweText": "Skriv in din säkerhetskod i BankIDappen och välj Legitimera eller Skriv under.",
        "engText": "Enter your security code in the BankID app and select Identify or Sign."
    }
}

A data class is a bit overkill for this and I would rather stay with dicts for simplicity, but it is no hard requirement.

The main reason that I did not implement this is that was too many niche cases to take into account and I felt that it would be a liability to support, so I postponed any work on it...

I do not use pybankid myself since a couple of years back, so I am not up to date with what needs to be done and changed. If you use it a lot and want to help maintain it, I can grant you commit rights on the repo so you can make changes and merge PRs yourself. I can still review the code if you want, but you are free to take pybankid in a direction you think is prudent.

[Majsvaffla]

I agree that the JSON data should not be touched. That's why I suggested a tuple or a class, even though it might be a bit overkill. At least i allows us to leave the response auntouched as an attribute.

I don't like the idea of adding the rfa key to the response as it becomes unclear which keys originate from the BankID API and which were added by pybankid.

There is a lot of cases to take into account for sure but at least it's a bit more straightforward now without the SOAP.

If we were to not return anything but the JSON response from the jsonclient methods I think there should be a function to resolve into a RFA; either a single function accepting both exceptions and hint-/error codes or two separate functions. What do you think about that?

Sure, I wouldn't mind to be granted commit rights. I'm currently using pybankid in a single project but I imagine it will remain so for quite some years.

[saqibur]

Hello. Is this being worked on or are there any plans to implement this somewhere down the line?

[Majsvaffla]

Hi

It is not being worked on. I haven't had any time to spend on it and the project where I'm using pybankid already has implemented all the RFAs.

[saqibur]

In that cause, would it be alright for me to take over this? I think it'll be quite handy to have the RFA hint codes as well - I myself have felt the need for it when using pybankid in multiple different projects.

[Majsvaffla]

Yes, you're welcome to submit a PR :) I'll assign the issue to you.