The problem here is not about casting to boolean, but about accessing .anonymous field of currentUserInfo. currentUserInfo does not have to contain this field so flow complained about it. using the ternary operator here was the least verbose way to satisfy flow.

I also didn't consider this case. It is good that you mention it. If transition between logged user is expected to go through some intermediate state when currentUserInfo is either null or contains anonymous field then the code above will work. But if we wish that this transition will be direct then we probably we will need to introduce changes. To summarise we want to fire clearSensitiveData everytime:

1. currentUserInfo becomes null or gets anonymous field set to true.
2. User opens the app and currentUserInfo is null or has anonymous field set to true.
3. currentUserInfo.id changes.

Probably 1 and 3 can be merged together to something like: currentUserInfo becomes null or changes its id field.

Just to clarify: we attempt to delete the db every time a user is anonymous instead of doing that only during log out process. To check if user is logging out, we would need to compare anonymous with previous state and check if it changed.
In this case this is intentional. We would like to handle the case when the app was force quitted during during logging out but before the db was deleted. Restarting the app would result in a state when a user is anonymous but the db contains the old data. So every time a user is anonymous we attempt to delete the db and not only when it's changing.

In the comment above Tomek explains why it is essential that we fire this function when the app is opened and the user is not logged in. In clearSensitiveData three things happen in C++ layer (in the following order):

1. Database is deleted (on condition it does exist).
2. Encryption key is overwritten.
3. New database is created - it is essential, since there are components that try to access database even if the user is logged out.

If 1 or 2 fails we are left with undeleted database and/or secure store - that is a safety threat. If 3) fails we are left with running application with deleted database, so it would eventually crash itself. So we intentionally kill the app if any of 1) - 3) fails and restart the process from the beginning when the user opens the app. That is why we want to run this function if nobody is logged in but the app is opened since we might need to recover from the failure of one of 1 - 3.

I will be currently working on new boolean conditions that will correctly handle those and only those cases.

I was asking more, does running clearSensitiveData like this cause any problems? Is clearSensitiveData able to handle being run when the app is opened and the user is not logged in? Has this case been tested?

As in previous version of this diff, it is not the case of casting to boolean. Flow complains if I try to access anonymous attribute since currentUserInfo might not have it. This is how I was able to satisfy flow. Another possibility was lambda with if statement that checks whether currentUserInfo as anonymous attribute. This one is less verbose.

Isn't the first condition a subset of the second condition?

Not exactly, second condition assumes currentUserInfo is not null. But they can be simplified to:
databaseCurrentUserInfoID && (userLoggedOut || currentUserInfo?.id !== databaseCurrentUserInfoID)

We want to run this effect also when currentUserInfo becomes null. But I am not react expert and might not know that passing currentUserInfo?.id instead will also cause this effect to be triggered if currentUserInfo changes to null.

Is clearSensitiveData able to handle being run when the app is opened and the user is not logged in?

This function is being executed when user opens the app but nobody is logged in. Previous comment in this thread should have explained this here:

That is why we want to run this function if nobody is logged in but the app is opened since we might need to recover from the failure of one of 1 - 3.

Has this case been tested?

This case is mentioned in the test plan for this diff:

Application should work without any difference no matter whether it is kept foregrounded, backgrounded or closed by the user between subsequent log-in and log-out actions.

I wouldn't put this diff on phabricator without executing steps and seeing the outcome as described in the test plan.

It is good that you pointed it out. I didn't notice it since it is not actually an issue not to do so. In most cases we will delete database after logout so currentUserInfo is null and we cannot call setCurrentUserID. But once we support direct transition from one user to another we should update this data in the database. So instead of placing a direct call to setCurrentUserID after clearSensitiveData I think it is best to convert the else if branch into separate if statement. Like this:

if (currentLoggedInUserID && currentLoggedInUserID !== databaseCurrentUserInfoID) {
    global.CommCoreModule.setCurrentUserID(currentLoggedInUserID);
}

It could be simplified to:

if (currentLoggedInUserID) {
    global.CommCoreModule.setCurrentUserID(currentLoggedInUserID);
}

But at the cost that setCurrentUserID will be called every time user opens the app while being already logged in, which will not be harmful but redundant.
Keeping in mind that this code is already quite complicated I will opt for the second option.

I don't think there are going to be any threading issues here. Both calls are implemented as blocking and running on the main thread on the C++ side.

I am not sure whether it is possible. The first one (global.CommCoreModule.clearSensitiveData()) does not execute any database query. It uses file deletion to clear database, then updates secure store with new encryption key, finally re-creates new empty database. See here: https://phab.comm.dev/D4728. The next global.CommCoreModule.setCurrentUserID(currentLoggedInUserID) is a regular database query. I don't think we can tie file system operations and SQLite query into a SQLite transaction.

Ah sorry didn't realize clearSensitiveData() was just a file operation... feel free to disregard what I said.