frontend/backend: pass final tx. note to sendtx and remove propose-tx-note handler
#2861
Conversation
|
(has a one line conflict with #2860) |
| err := handlers.account.SendTx() | ||
| var finalNote string | ||
| if err := json.NewDecoder(r.Body).Decode(&finalNote); err != nil { | ||
| handlers.log.WithError(err).Error("Failed to unmarshal transaction note") |
There was a problem hiding this comment.
note: I decided to ignore the error on purpose here and only log it. it is better if users contact us with a bug that their notes do not get applied rather then I can't send any coins anymore
this should not happen anyways, and if it does proceeding with the zero string value is like the user never entered anything into the note field.
There was a problem hiding this comment.
*what I mean is I don't return an error here and continue on purpose
There was a problem hiding this comment.
please add this as a comment, otherwise the next person to look at this will think it's a bug and return an error 😄
NicolaLS
force-pushed
the
pass-note-to-sendtx
branch
from
fd002f4
to
43638d2
Compare
| err := handlers.account.SendTx() | ||
| var finalNote string | ||
| if err := json.NewDecoder(r.Body).Decode(&finalNote); err != nil { | ||
| handlers.log.WithError(err).Error("Failed to unmarshal transaction note") |
There was a problem hiding this comment.
please add this as a comment, otherwise the next person to look at this will think it's a bug and return an error 😄
|
Also it should be one commit, it's one logical change. |
NicolaLS
force-pushed
the
pass-note-to-sendtx
branch
from
43638d2
to
fc49d09
Compare
|
(rebased) |
Pass the final transaction note to the sendtx handler on submit (send) instead of remembering a proposed transaction note which is set separately using the propose-tx-note handler. Currently there is no benefit of having a separate handler for the transaction note for the active proposal. It was separated in this commit because typing in the note field would cause the other fields to be validated/calculated: 4eb692c However since the backend does not perform any calculations or validation on this handler, the final note can be submitted when clicking send (calling the sendtx handler).
NicolaLS
force-pushed
the
pass-note-to-sendtx
branch
from
fc49d09
to
bde57c4
Compare
|
@benma thanks :) I rebased, squashed, and added a comment explaining why we don't return an error when unmarshaling the tx. note fails, PTAL. |
Noticed this when planning the refactor of (frontend)
send.tsx, this PR does not change anything and should be a pure refactor.propose-tx-notehandler did nothing but setting a field onBaseAccountit did not validate anything, thus removing it and just using the final tx note reduces the code size but changes nothing.We initially separated the note from the txProposal because we did not want to perform any actions when (e.g calculate fee) when user types a note:
4eb692c
It would be good to validate the note though to match the constrains here, but that would not be a refactor so maybe in another PR:
bitbox-wallet-app/backend/accounts/notes/notes.go
Line 92 in 87d6219