search

consento-org / mobile

The first (currently alpha) Consento mobile application
https://consento.org
MIT License
14 stars 1 forks source link

Updates #93

Closed martinheidegger closed 4 years ago

martinheidegger commented 4 years ago

As part of a security refactoring done with @consento-org/notifications-server this PR updates a whole lot of dependencies. Including the latest version of expo and an updated, more stable notification system.

This is a breaking update that will effectively hide the vaults from users from the current app. Users of the current App that have essential data in their system will be able to simply start an older version (through npm or expo links) to download current data in the vaults, though all relations will be lost.

Task list --- - [x] Use the new notification server - [x] Test connection resets - [x] Update to expo@~~38~~39 - [x] fix warning about missing splash screen - [x] Use `get-random-values-polypony` - [x] Use react-navigation@5 (in progress) - [x] Use typescript strict mode (in progress) - [x] Fix problem with state user interface that prevents saving of changes - [x] Use [VirtualizedList](https://reactnative.dev/docs/virtualizedlist) for the vault grid - [x] Update to latest `expo-export` (~~should be minor~~ turned out to be major with the changes in 4 & 5) - [x] Vaults - [x] Vault - [x] FileList - [x] Locks - [x] Log - [x] Fix unlocking - [x] Config - [x] Fix wrong "updating" display - [x] Text Editor - [x] Image Editor - [x] Consentos - [x] Relations - [x] Relation - [x] New Relation
martinheidegger commented 4 years ago

Having tested it to a reasonable degree for a few days and now released all parts of it (including the notification-server). The PR now contains only one (squashed) commit https://github.com/consento-org/mobile/pull/93/commits/6ce40bb87780f59fcd371cc1efe702bc8ba207bd that explains the history a bit.

I am hoping for @dkastl or @RangerMauve to review it for general functionality (has it broken important things) and does it still works. Future refactorings in the model layer and stability work on the notification system will need to be postponed to future releases. My open is that the PR can be merged after asserting if it basically works and doesn't break too much.

dkastl commented 4 years ago

Is this a known issue? https://github.com/consento-org/mobile/issues/98

dkastl commented 4 years ago

Good idea to use newer version of expo-cli as in #100 ?

martinheidegger commented 4 years ago

@dkastl the current version should have mostly updated dependencies (some still outstanding for they might have unintended consequences). Also, as visible in the changelog, many, many bugs and improved a display of error messages. Please run npm start -- --clear to make sure that the latest node_modules are used before a test/review.

Note: the expo builds are broken for the moment, due to updates in how the screenshots are taken. Will update the expo build scripts tomorrow.

dkastl commented 4 years ago

What is the goal of this pull request? It probably makes sense to get it merged at some point even if it isn't perfect.

I tested it with two devices and this worked:

However, there seem to be issues, like right after locking the vault an unlock request is sent again. But maybe we are going to change things anyway, so let's not try to wait until everything is perfect.

martinheidegger commented 4 years ago

What is the goal of this pull request?

The goal is to have a reasonably working mobile app (again) that can be shown to eventual users that uses updated libraries and metaphors (i.e. new expo). The goal is not zero bugs, though it would be good to not have old features broken if possible.

However, there seem to be issues, like right after locking the vault an unlock request is sent again.

Can you reproduce this?

dkastl commented 4 years ago

Can you reproduce this?

Probably, but I had to start from beginning with some sort of checklist ... and I need a bit of time. If you find it important to be solved with this PR, then I will describe it in another issue.

martinheidegger commented 4 years ago

I managed to get the same error once by accident but wasn't able to get it consistently which suggest that it may have happened before as well without us noticing. However: if it happens somehow consistently then its likely a new bug that has been introduced and probably needs some looking after. When I have the expo ci running and you think the app is good enough to show in case someone is interested than that state is fine with me. (it definitely wasn't in that state a week ago).

dkastl commented 4 years ago

Yes, I'm not sure either, if it's something new or if it has happened already before. Will test tonight in case INPHO summit is as exciting as last night ;-)

martinheidegger commented 4 years ago

So the last PR finishes the one bug found in #102 merging this PR for now, lets look forward.