to add auto confirm for other currencies in the future. The generic part
is only used where we would have issues with backward compatibility like
in the protobuf objects. Most of the current classes are kept XMR
specific and could be generalized once we add other assets, but that
would be an internal refactoring without breaking any network or
storage data. I think it would be premature to go further here as we
don't know the details of other use cases. I added the methods used from
clients to AutoConfirmResult, not sure if the API is well defined by
that, but as said that could become subject of a future refactoring once
another auto confirm feature gets added. Goal of that refactoring was
to avoid that we need more fields for trade and the the UI would have to
deal with lots of switch cases based on currency.
Sorry that is a larger commit, would have been hard to break up...
- Replace httpClient.toString() with httpClient.getBaseUrl() as toString
would deliver too much for those use cases.
- Add @Nullable
- Log improvements
instead which stores the stateName. [1]
- Adjust protobuf methods
- Add UNDEFINED to AutoConfirmResult.State to support cases where we
get no data to set the enum.
- Add NO_MATCH_FOUND (used in follow up commits)
- Refactoring: Improve constructors
[1]
Enums in protobuf are not well supported. They are global so an enum
with name (e.g. State) inside Trade conflicts with another enum inside
Message with the same name. So they do not reflect encapsulation in the
class like in java.
We moved over time to the strategy to use strings (from enum.name())
instead of the enum, avoiding also cumbersome fromProto and toProto
code and being more flexible with updates.
The autoConfirmResultState enum inside Trade was a bit confusing to me
as it was a different structure as in the java code. We try to mirror
the structure as far as possible.
If a user has an existing account with phone number or email as
account ID we show a popup at startup where we require that he sets the
user name. This popup has no close button so he is forced to enter a
value. If there are multiple account multiple popups will be shown.
To not break signed accounts we keep accountId as internal id used for signing.
Old accounts get a popup to add the new required field userName but accountId is
left unchanged. Newly created accounts fill accountId with the value of userName.
In the UI we only use userName.
Input validation does only check for length (5-100 chars). Not sure what
are the requirements at Revolut. Can be changes easily if anyone gets
the specs.
I don't know why the tests failed as I just added an overloaded method
and it should not have any impact. There is also one exception which
makes it even more obscure. I guess its some test framework issue.
See comment at the exceptional handling
// If we remove the last argument (isNull()) tests fail. No idea why as the broadcast method has an
/ overloaded method with nullable listener. Seems a testframework issue as it should not matter if the
// method with listener is called with null argument or the other method with no listener. We removed the
// null value from all other calls but here we can't as it breaks the test.
