python-fints icon indicating copy to clipboard operation
python-fints copied to clipboard

Add a full code example to the docs if a TAN is required

Open kmille opened this issue 4 years ago • 6 comments

Hey, I just added some docs. What still confuses me is that the docs state for the send_tan function:

Returns: | Currently no response

Source: https://python-fints.readthedocs.io/en/latest/tans.html#sending-the-tan

kmille avatar Jan 05 '21 16:01 kmille

Codecov Report

Merging #124 (9975a4e) into master (bc1c81e) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   87.48%   87.48%           
=======================================
  Files          23       23           
  Lines        3117     3117           
=======================================
  Hits         2727     2727           
  Misses        390      390           

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 bc1c81e...9975a4e. Read the comment docs.

codecov[bot] avatar Jan 05 '21 16:01 codecov[bot]

I updated and tested (without TAN, SMS-TAN and flicker TAN) the PR.

kmille avatar Jan 12 '21 17:01 kmille

Ready to merge or still some suggestions?

kmille avatar Jan 25 '21 19:01 kmille

Hey, small merge reminder :) I think there is something wrong with the tests (It only didn't run on Python 3.4).

kmille avatar Apr 03 '21 11:04 kmille

Very helpful addition! Thanks! It saved my day. Two improvement ideas:

  1. If there is only one TAN-method available, just set it without bothering the user with a question.
  2. A more simple example for the common case of getting the TAN by SMS would be nicer:
if isinstance(result, NeedTANResponse):
    tan = input('Please enter TAN:')
    result = client.send_tan(result, tan.strip())

ogai avatar Aug 06 '21 11:08 ogai

Hey @raphaelm still interested in merging?

kmille avatar Sep 27 '21 09:09 kmille