remove logic from ChatMessage

[mahibi]

the talk-android/app/src/main/java/com/nextcloud/talk/models/json/chat/ChatMessage.java contains much logic but it should only be a data class.

Especially the selectedIndividualHashMap must be refactored (it is somehow filled in getImageUrl() which is totally weird). So accessing the selectedIndividualHashMap before getImageUrl() is called results in errors. (i guess https://github.com/nextcloud/talk-android/issues/1510#issuecomment-909531062 is an example for this problem)

I was tempted to clean up there but i think we should first talk more about the architecture where the logic should be moved to. I think we should discuss this together @AlvaroBrey @timkrueger @tobiasKaminsky @AndyScherzinger

[mahibi]

regarding the selectedIndividualHashMap stuff: what is the best way to access the messageParameters (see https://nextcloud-talk.readthedocs.io/en/latest/chat/#receive-chat-messages-of-a-conversation) and where should it be done?

also to be compared with the new approach from com/nextcloud/talk/viewmodels/SharedItemsViewModel.kt:66 (but this might still not be the best solution if i remember correctly from todays dicussion @AlvaroBrey & @timkrueger ?)

[tobiasKaminsky]

what is the best way to access the messageParameters (see https://nextcloud-talk.readthedocs.io/en/latest/chat/#receive-chat-messages-of-a-conversation) and where should it be done?

This can be similar to SharedItemRepository, something like ChatRepository.

Especially the selectedIndividualHashMap must be refactored (it is somehow filled in getImageUrl() which is totally weird). So accessing the selectedIndividualHashMap before getImageUrl() is called results in errors.

It seems that this is just a temporary property to make it faster, as e.g. hasFileAttachment() is using same approach, but aways iterates over entire messageParameters map.

If selectedIndividualHashMap is needed you can set this within messageParameters, or have a dynamic getter for selectedIndividualHashMap, which returns correct map (or null).

[AlvaroBrey]

regarding the selectedIndividualHashMap stuff: what is the best way to access the messageParameters (see https://nextcloud-talk.readthedocs.io/en/latest/chat/#receive-chat-messages-of-a-conversation) and where should it be done?

From a quick search it seems that this map is only written here, but heavily read outside the ChatMessage class. To me this is an indicator that this map should, at the very least, be initialized when the ChatMessage is constructed. As the comment there says, it should very much not be initialized in a getter.

In general, for this class, I can see a lot of getters that could instead be immutable properties of a ChatMessage object. This map is one of them, but also getMessageType(), getImageUrl(), hasGeoLocation() and many others.

From a separation of concerns standpoint, when we fetch ChatMessages from the API (model.json.ChatMessage), I would map them into another class (model.ChatMessage) which holds only the data, without any network annotations or extra logic. The specific way in which this is done is not that relevant (ideally we'd have a Repository as @tobiasKaminsky said); the important part is to make the data class smaller and with a single responsibility.