[bisq-network/bisq] PaymentAccount object removal from ObservableSet issue (#3572)

[Christoph Atteneder]

ripcurlx requested changes on this pull request.

NACK
> After this change i could remove uphold/revolut without any issue.
> 
> What i believe is because of hashcode change, we are not able to remove. hashcode change can be because of some state change. so usually we restrict that to unique identifying fields (id, date in this case) to maintain hashcode contract.
> 
> I couldn't create moneygram account even in prod. will open a bug. #3598

But as @chimp1984 mentioned above I'm not sure we should fix it like that with only using id and date.  The account name cannot be changed in the interface so this can't be the problem of this bug IMO. Probably this part of the code doesn't need changes, but more the `removePaymentAccount` method in the User class as @chimp1984 pointed out. We have to find out why ObservableSet is behaving not as expected in that case.

> ```
>   public void removePaymentAccount(PaymentAccount paymentAccount) {
>         HashSet<PaymentAccount> temp = new HashSet<>(paymentAccountsAsObservable);
>         boolean changed = temp.remove(paymentAccount);
>         paymentAccountsAsObservable.clear();
>         paymentAccountsAsObservable.addAll(temp);
>         if (changed)
>             persist();
>     }
> ```





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3572#pullrequestreview-317085296
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191114/599cfb7c/attachment.html>