View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0008552 | Taler | wallet (Android App) | public | 2024-02-29 23:25 | 2024-04-15 19:32 |
| Reporter | Florian Dold | Assigned To | avalos | Resolution deadline | |
| Implementer | Reviewer | ||||
| Release signed by | Deployed by | ||||
| Verifier | Communicator | ||||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | urgent | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | reopened | ||
| Product Version | git (master) | ||||
| Target Version | 0.10 | Fixed in Version | 0.10 | ||
| Summary | 0008552: Android wallet does not render peer-push-debit transaction in state pending(refresh-expired) correctly | ||||
| Description | Error message is: Error: net.taler.wallet.transactions.TransactionMinorState does not contain element with name "refresh-expired". In general, the wallet UI should probably be more lenient about minor states it doesn't know. | ||||
| Tags | No tags attached. | ||||
| I don't think it's relevant for the UI to handle unknown major/minor states better, as they're part of wallet-core and thus the wallet is expected to always implement all of the states of a given wallet-core version. Those breaking changes should always be reported. In this case, I didn't know that there was a new `refresh-expired` state. How should I render this state? | |
| There should be some default logic for unknown minor states. The minor state `refresh-expired` isn't new, it's just something that doesn't occur very often. For new major states, I agree that we should bump the protocol version in that case. | |
| I don't remember `refresh-expired` being anywhere when I first implemented DD37, so it must be new even if it's not recent. I just added `refresh-expired` to the enum, and with that the app should cover all current major/minor states currently present in wallet-core. | |
| Ah, but this is about the app not exploding when there's a new minor state. Could we at least allow the default actions and show the state name? | |
| I think that both new minor and major states should always be reported. Since the initial DD37 implementation, there has only been one new minor state introduced in wallet-core, so I don't see a reason to worry that much about the wallet breaking with new minor states, as long as they are reported properly. | |
| Also, the app doesn't decide which actions to allow, that's on wallet-core. The state name is never shown, should I implement that as a developer feature? It would be nice to show the full JSON, but that should be a separate ticket. | |
| The issue has been resolved in https://git.taler.net/taler-android.git/commit/?id=917a0175af8493b893486c0b6b2a001ce1b952c6 > this is about the app not exploding when there's a new minor state. Note that Android already has a custom TransactionListSerializer (that I am also not a big fan of), because wallet-core kept breaking the API and there was the desire to still render transactions unknown to Android. Because of this, the app should not have exploded, but shown the transaction as unknown. | |
| What would help preventing such issues in the future is properly announcing breaking changes to the API and to expose all possible states to wallet-core users for testing e.g. through the integration test. Then, we cold press one button and check the UI for any breakage after updating a new wallet-core version. | |
| I will close this issue already. The crash has been fixed already. The question of whether unknown minor states should crash the app or not, remains open, but I think that we should discuss this separately. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-02-29 23:25 | Florian Dold | New Issue | |
| 2024-02-29 23:25 | Florian Dold | Status | new => assigned |
| 2024-02-29 23:25 | Florian Dold | Assigned To | => avalos |
| 2024-03-01 03:55 | avalos | Note Added: 0021593 | |
| 2024-03-04 15:17 | avalos | Assigned To | avalos => Florian Dold |
| 2024-03-04 15:17 | avalos | Status | assigned => feedback |
| 2024-03-04 16:05 | Florian Dold | Note Added: 0021645 | |
| 2024-03-04 16:05 | Florian Dold | Status | feedback => informing |
| 2024-03-04 16:05 | Florian Dold | Resolution | open => fixed |
| 2024-03-05 15:50 | avalos | Assigned To | Florian Dold => avalos |
| 2024-03-05 15:50 | avalos | Status | informing => feedback |
| 2024-03-05 15:50 | avalos | Resolution | fixed => reopened |
| 2024-03-05 15:51 | avalos | Assigned To | avalos => Florian Dold |
| 2024-03-05 15:51 | avalos | Assigned To | Florian Dold => avalos |
| 2024-03-05 16:18 | avalos | Note Added: 0021687 | |
| 2024-03-05 20:12 | Florian Dold | Status | feedback => assigned |
| 2024-03-05 20:44 | Florian Dold | Note Added: 0021696 | |
| 2024-03-07 16:59 | avalos | Note Added: 0021764 | |
| 2024-03-07 17:04 | avalos | Note Added: 0021766 | |
| 2024-03-07 19:41 | Christian Grothoff | Priority | normal => urgent |
| 2024-03-07 19:41 | Christian Grothoff | Product Version | => git (master) |
| 2024-03-07 19:41 | Christian Grothoff | Target Version | 0.9.4 => 0.10 |
| 2024-03-13 17:58 | grote | Note Added: 0021899 | |
| 2024-03-13 18:04 | grote | Note Added: 0021900 | |
| 2024-03-21 17:23 | avalos | Status | assigned => informing |
| 2024-03-21 17:23 | avalos | Note Added: 0021964 | |
| 2024-04-09 11:11 | Christian Grothoff | Fixed in Version | => 0.10 |
| 2024-04-15 19:32 | Christian Grothoff | Status | informing => closed |