From 3a274b0594663cbca9d025d93beb026d34f61285 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Fri, 22 Sep 2023 15:17:38 +0200 Subject: [PATCH] refactor: Replace .to... with .from() in most cases (#82) The previous code generally converted between a higher and a lower type by putting the type conversion functions on the lower type. This introduced cycles in the code dependency graph, and made it more difficult to follow the code flow. Refactor the code so that types generally have a `from(...)` static factory method that can create an instance from a lower type, and if appropriate a `to...()` method that can also create an instance of that lower type. Add `docs/code-style.md` which explains the rationale for this change in more detail so that future contributors can write code in the same style. --- .../pachli/adapter/StatusBaseViewHolder.java | 39 +-- .../conversation/ConversationEntity.kt | 166 +++++-------- .../conversation/ConversationViewData.kt | 47 +--- .../ConversationsRemoteMediator.kt | 14 +- .../conversation/ConversationsViewModel.kt | 16 +- .../notifications/NotificationsViewModel.kt | 4 +- .../components/report/ReportViewModel.kt | 4 +- .../report/adapter/StatusViewHolder.kt | 7 +- .../components/search/SearchViewModel.kt | 4 +- .../timeline/TimelineTypeMappers.kt | 235 ------------------ .../viewmodel/CachedTimelineRemoteMediator.kt | 13 +- .../viewmodel/CachedTimelineViewModel.kt | 8 +- .../viewmodel/NetworkTimelineViewModel.kt | 4 +- .../viewmodel/TrendingTagsViewModel.kt | 12 +- .../viewthread/ViewThreadViewModel.kt | 112 +++++---- .../viewthread/edits/ViewEditsAdapter.kt | 4 +- .../app/pachli/db/TimelineStatusEntity.kt | 206 ++++++++++++++- .../java/app/pachli/util/StatusViewHelper.kt | 35 ++- .../java/app/pachli/util/ViewDataUtils.kt | 97 -------- .../pachli/viewdata/NotificationViewData.kt | 27 +- .../java/app/pachli/viewdata/PollViewData.kt | 49 ++-- .../app/pachli/viewdata/StatusViewData.kt | 112 +++++++++ .../app/pachli/viewdata/TrendingViewData.kt | 15 ++ .../components/timeline/StatusMocker.kt | 8 +- docs/code-style.md | 122 +++++++++ docs/contributing/code.md | 50 ++-- 26 files changed, 762 insertions(+), 648 deletions(-) delete mode 100644 app/src/main/java/app/pachli/components/timeline/TimelineTypeMappers.kt delete mode 100644 app/src/main/java/app/pachli/util/ViewDataUtils.kt create mode 100644 docs/code-style.md diff --git a/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.java b/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.java index 7c7c9a72c..4dfe5fbc9 100644 --- a/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.java +++ b/app/src/main/java/app/pachli/adapter/StatusBaseViewHolder.java @@ -53,6 +53,7 @@ import app.pachli.entity.Emoji; import app.pachli.entity.Filter; import app.pachli.entity.FilterResult; import app.pachli.entity.HashTag; +import app.pachli.entity.Poll; import app.pachli.entity.Status; import app.pachli.interfaces.StatusActionListener; import app.pachli.util.AbsoluteTimeFormatter; @@ -279,7 +280,7 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder { List mentions = actionable.getMentions(); List tags =actionable.getTags(); List emojis = actionable.getEmojis(); - PollViewData poll = PollViewDataKt.toViewData(actionable.getPoll()); + Poll poll = actionable.getPoll(); if (expanded) { CharSequence emojifiedText = CustomEmojiHelper.emojify(content, emojis, this.content, statusDisplayOptions.animateEmojis()); @@ -288,7 +289,7 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder { updateMediaLabel(i, sensitive, true); } if (poll != null) { - setupPoll(poll, emojis, statusDisplayOptions, listener); + setupPoll(PollViewData.Companion.from(poll), emojis, statusDisplayOptions, listener); } else { hidePoll(); } @@ -962,24 +963,28 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder { private CharSequence getPollDescription(@NonNull StatusViewData status, @NonNull Context context, @NonNull StatusDisplayOptions statusDisplayOptions) { - PollViewData poll = PollViewDataKt.toViewData(status.getActionable().getPoll()); + Poll poll = status.getActionable().getPoll(); if (poll == null) { return ""; - } else { - Object[] args = new CharSequence[5]; - List options = poll.getOptions(); - for (int i = 0; i < args.length; i++) { - if (i < options.size()) { - int percent = PollViewDataKt.calculatePercent(options.get(i).getVotesCount(), poll.getVotersCount(), poll.getVotesCount()); - args[i] = buildDescription(options.get(i).getTitle(), percent, options.get(i).getVoted(), context); - } else { - args[i] = ""; - } - } - args[4] = getPollInfoText(System.currentTimeMillis(), poll, statusDisplayOptions, - context); - return context.getString(R.string.description_poll, args); } + + PollViewData pollViewData = PollViewData.Companion.from(poll); + Object[] args = new CharSequence[5]; + List options = pollViewData.getOptions(); + int totalVotes = pollViewData.getVotesCount(); + Integer totalVoters = pollViewData.getVotersCount(); + + for (int i = 0; i < args.length; i++) { + if (i < options.size()) { + int percent = PollViewDataKt.calculatePercent(options.get(i).getVotesCount(), totalVoters, totalVotes); + args[i] = buildDescription(options.get(i).getTitle(), percent, options.get(i).getVoted(), context); + } else { + args[i] = ""; + } + } + args[4] = getPollInfoText(System.currentTimeMillis(), pollViewData, statusDisplayOptions, + context); + return context.getString(R.string.description_poll, args); } @NonNull diff --git a/app/src/main/java/app/pachli/components/conversation/ConversationEntity.kt b/app/src/main/java/app/pachli/components/conversation/ConversationEntity.kt index ad09226f0..963dbee23 100644 --- a/app/src/main/java/app/pachli/components/conversation/ConversationEntity.kt +++ b/app/src/main/java/app/pachli/components/conversation/ConversationEntity.kt @@ -26,7 +26,6 @@ import app.pachli.entity.HashTag import app.pachli.entity.Poll import app.pachli.entity.Status import app.pachli.entity.TimelineAccount -import app.pachli.viewdata.StatusViewData import java.util.Date @Entity(primaryKeys = ["id", "accountId"]) @@ -39,13 +38,26 @@ data class ConversationEntity( val unread: Boolean, @Embedded(prefix = "s_") val lastStatus: ConversationStatusEntity, ) { - fun toViewData(): ConversationViewData { - return ConversationViewData( - id = id, + companion object { + fun from( + conversation: Conversation, + accountId: Long, + order: Int, + expanded: Boolean, + contentShowing: Boolean, + contentCollapsed: Boolean, + ) = ConversationEntity( + accountId = accountId, + id = conversation.id, order = order, - accounts = accounts, - unread = unread, - lastStatus = lastStatus.toViewData(), + accounts = conversation.accounts.map { ConversationAccountEntity.from(it) }, + unread = conversation.unread, + lastStatus = ConversationStatusEntity.from( + conversation.lastStatus!!, + expanded = expanded, + contentShowing = contentShowing, + contentCollapsed = contentCollapsed, + ), ) } } @@ -70,6 +82,17 @@ data class ConversationAccountEntity( emojis = emojis, ) } + + companion object { + fun from(timelineAccount: TimelineAccount) = ConversationAccountEntity( + id = timelineAccount.id, + localUsername = timelineAccount.localUsername, + username = timelineAccount.username, + displayName = timelineAccount.name, + avatar = timelineAccount.avatar, + emojis = timelineAccount.emojis.orEmpty(), + ) + } } @TypeConverters(Converters::class) @@ -100,104 +123,37 @@ data class ConversationStatusEntity( val language: String?, ) { - fun toViewData(): StatusViewData { - return StatusViewData( - status = Status( - id = id, - url = url, - account = account.toAccount(), - inReplyToId = inReplyToId, - inReplyToAccountId = inReplyToAccountId, - content = content, - reblog = null, - createdAt = createdAt, - editedAt = editedAt, - emojis = emojis, - reblogsCount = 0, - favouritesCount = favouritesCount, - repliesCount = repliesCount, - reblogged = false, - favourited = favourited, - bookmarked = bookmarked, - sensitive = sensitive, - spoilerText = spoilerText, - visibility = Status.Visibility.DIRECT, - attachments = attachments, - mentions = mentions, - tags = tags, - application = null, - pinned = false, - muted = muted, - poll = poll, - card = null, - language = language, - filtered = null, - ), - isExpanded = expanded, - isShowingContent = showingHiddenContent, - isCollapsed = collapsed, + companion object { + fun from( + status: Status, + expanded: Boolean, + contentShowing: Boolean, + contentCollapsed: Boolean, + ) = ConversationStatusEntity( + id = status.id, + url = status.url, + inReplyToId = status.inReplyToId, + inReplyToAccountId = status.inReplyToAccountId, + account = ConversationAccountEntity.from(status.account), + content = status.content, + createdAt = status.createdAt, + editedAt = status.editedAt, + emojis = status.emojis, + favouritesCount = status.favouritesCount, + repliesCount = status.repliesCount, + favourited = status.favourited, + bookmarked = status.bookmarked, + sensitive = status.sensitive, + spoilerText = status.spoilerText, + attachments = status.attachments, + mentions = status.mentions, + tags = status.tags, + showingHiddenContent = contentShowing, + expanded = expanded, + collapsed = contentCollapsed, + muted = status.muted ?: false, + poll = status.poll, + language = status.language, ) } } - -fun TimelineAccount.toEntity() = - ConversationAccountEntity( - id = id, - localUsername = localUsername, - username = username, - displayName = name, - avatar = avatar, - emojis = emojis.orEmpty(), - ) - -fun Status.toEntity( - expanded: Boolean, - contentShowing: Boolean, - contentCollapsed: Boolean, -) = - ConversationStatusEntity( - id = id, - url = url, - inReplyToId = inReplyToId, - inReplyToAccountId = inReplyToAccountId, - account = account.toEntity(), - content = content, - createdAt = createdAt, - editedAt = editedAt, - emojis = emojis, - favouritesCount = favouritesCount, - repliesCount = repliesCount, - favourited = favourited, - bookmarked = bookmarked, - sensitive = sensitive, - spoilerText = spoilerText, - attachments = attachments, - mentions = mentions, - tags = tags, - showingHiddenContent = contentShowing, - expanded = expanded, - collapsed = contentCollapsed, - muted = muted ?: false, - poll = poll, - language = language, - ) - -fun Conversation.toEntity( - accountId: Long, - order: Int, - expanded: Boolean, - contentShowing: Boolean, - contentCollapsed: Boolean, -) = - ConversationEntity( - accountId = accountId, - id = id, - order = order, - accounts = accounts.map { it.toEntity() }, - unread = unread, - lastStatus = lastStatus!!.toEntity( - expanded = expanded, - contentShowing = contentShowing, - contentCollapsed = contentCollapsed, - ), - ) diff --git a/app/src/main/java/app/pachli/components/conversation/ConversationViewData.kt b/app/src/main/java/app/pachli/components/conversation/ConversationViewData.kt index 391477e84..f379f742c 100644 --- a/app/src/main/java/app/pachli/components/conversation/ConversationViewData.kt +++ b/app/src/main/java/app/pachli/components/conversation/ConversationViewData.kt @@ -25,7 +25,7 @@ data class ConversationViewData( val unread: Boolean, val lastStatus: StatusViewData, ) { - fun toEntity( + fun toConversationEntity( accountId: Long, favourited: Boolean = lastStatus.status.favourited, bookmarked: Boolean = lastStatus.status.bookmarked, @@ -52,41 +52,14 @@ data class ConversationViewData( ), ) } -} -fun StatusViewData.toConversationStatusEntity( - favourited: Boolean = status.favourited, - bookmarked: Boolean = status.bookmarked, - muted: Boolean = status.muted ?: false, - poll: Poll? = status.poll, - expanded: Boolean = isExpanded, - collapsed: Boolean = isCollapsed, - showingHiddenContent: Boolean = isShowingContent, -): ConversationStatusEntity { - return ConversationStatusEntity( - id = id, - url = status.url, - inReplyToId = status.inReplyToId, - inReplyToAccountId = status.inReplyToAccountId, - account = status.account.toEntity(), - content = status.content, - createdAt = status.createdAt, - editedAt = status.editedAt, - emojis = status.emojis, - favouritesCount = status.favouritesCount, - repliesCount = status.repliesCount, - favourited = favourited, - bookmarked = bookmarked, - sensitive = status.sensitive, - spoilerText = status.spoilerText, - attachments = status.attachments, - mentions = status.mentions, - tags = status.tags, - showingHiddenContent = showingHiddenContent, - expanded = expanded, - collapsed = collapsed, - muted = muted, - poll = poll, - language = status.language, - ) + companion object { + fun from(conversationEntity: ConversationEntity) = ConversationViewData( + id = conversationEntity.id, + order = conversationEntity.order, + accounts = conversationEntity.accounts, + unread = conversationEntity.unread, + lastStatus = StatusViewData.from(conversationEntity.lastStatus), + ) + } } diff --git a/app/src/main/java/app/pachli/components/conversation/ConversationsRemoteMediator.kt b/app/src/main/java/app/pachli/components/conversation/ConversationsRemoteMediator.kt index fd1b5a3da..497eddffe 100644 --- a/app/src/main/java/app/pachli/components/conversation/ConversationsRemoteMediator.kt +++ b/app/src/main/java/app/pachli/components/conversation/ConversationsRemoteMediator.kt @@ -58,17 +58,13 @@ class ConversationsRemoteMediator( conversations .filterNot { it.lastStatus == null } .map { conversation -> - - val expanded = activeAccount.alwaysOpenSpoiler - val contentShowing = activeAccount.alwaysShowSensitiveMedia || !conversation.lastStatus!!.sensitive - val contentCollapsed = true - - conversation.toEntity( + ConversationEntity.from( + conversation, accountId = activeAccount.id, order = order++, - expanded = expanded, - contentShowing = contentShowing, - contentCollapsed = contentCollapsed, + expanded = activeAccount.alwaysOpenSpoiler, + contentShowing = activeAccount.alwaysShowSensitiveMedia || !conversation.lastStatus!!.sensitive, + contentCollapsed = true, ) }, ) diff --git a/app/src/main/java/app/pachli/components/conversation/ConversationsViewModel.kt b/app/src/main/java/app/pachli/components/conversation/ConversationsViewModel.kt index 1fa9c8a23..b92b77b05 100644 --- a/app/src/main/java/app/pachli/components/conversation/ConversationsViewModel.kt +++ b/app/src/main/java/app/pachli/components/conversation/ConversationsViewModel.kt @@ -55,14 +55,14 @@ class ConversationsViewModel @Inject constructor( ) .flow .map { pagingData -> - pagingData.map { conversation -> conversation.toViewData() } + pagingData.map { conversation -> ConversationViewData.from(conversation) } } .cachedIn(viewModelScope) fun favourite(favourite: Boolean, conversation: ConversationViewData) { viewModelScope.launch { timelineCases.favourite(conversation.lastStatus.id, favourite).fold({ - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, favourited = favourite, ) @@ -77,7 +77,7 @@ class ConversationsViewModel @Inject constructor( fun bookmark(bookmark: Boolean, conversation: ConversationViewData) { viewModelScope.launch { timelineCases.bookmark(conversation.lastStatus.id, bookmark).fold({ - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, bookmarked = bookmark, ) @@ -93,7 +93,7 @@ class ConversationsViewModel @Inject constructor( viewModelScope.launch { timelineCases.voteInPoll(conversation.lastStatus.id, conversation.lastStatus.status.poll?.id!!, choices) .fold({ poll -> - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, poll = poll, ) @@ -107,7 +107,7 @@ class ConversationsViewModel @Inject constructor( fun expandHiddenStatus(expanded: Boolean, conversation: ConversationViewData) { viewModelScope.launch { - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, expanded = expanded, ) @@ -117,7 +117,7 @@ class ConversationsViewModel @Inject constructor( fun collapseLongStatus(collapsed: Boolean, conversation: ConversationViewData) { viewModelScope.launch { - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, collapsed = collapsed, ) @@ -127,7 +127,7 @@ class ConversationsViewModel @Inject constructor( fun showContent(showing: Boolean, conversation: ConversationViewData) { viewModelScope.launch { - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, showingHiddenContent = showing, ) @@ -158,7 +158,7 @@ class ConversationsViewModel @Inject constructor( !(conversation.lastStatus.status.muted ?: false), ) - val newConversation = conversation.toEntity( + val newConversation = conversation.toConversationEntity( accountId = accountManager.activeAccount!!.id, muted = !(conversation.lastStatus.status.muted ?: false), ) diff --git a/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt b/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt index 9b205c29f..8c00158cd 100644 --- a/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/app/pachli/components/notifications/NotificationsViewModel.kt @@ -47,7 +47,6 @@ import app.pachli.util.StatusDisplayOptions import app.pachli.util.deserialize import app.pachli.util.serialize import app.pachli.util.throttleFirst -import app.pachli.util.toViewData import app.pachli.viewdata.NotificationViewData import app.pachli.viewdata.StatusViewData import at.connyduck.calladapter.networkresult.getOrThrow @@ -534,7 +533,8 @@ class NotificationsViewModel @Inject constructor( .map { pagingData -> pagingData.map { notification -> val filterAction = notification.status?.actionableStatus?.let { filterModel.shouldFilterStatus(it) } ?: Filter.Action.NONE - notification.toViewData( + NotificationViewData.from( + notification, isShowingContent = statusDisplayOptions.value.showSensitiveMedia || !(notification.status?.actionableStatus?.sensitive ?: false), isExpanded = statusDisplayOptions.value.openSpoiler, diff --git a/app/src/main/java/app/pachli/components/report/ReportViewModel.kt b/app/src/main/java/app/pachli/components/report/ReportViewModel.kt index 948d3b3f3..c2f1d5757 100644 --- a/app/src/main/java/app/pachli/components/report/ReportViewModel.kt +++ b/app/src/main/java/app/pachli/components/report/ReportViewModel.kt @@ -35,7 +35,7 @@ import app.pachli.util.Error import app.pachli.util.Loading import app.pachli.util.Resource import app.pachli.util.Success -import app.pachli.util.toViewData +import app.pachli.viewdata.StatusViewData import at.connyduck.calladapter.networkresult.fold import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.MutableSharedFlow @@ -79,7 +79,7 @@ class ReportViewModel @Inject constructor( .map { pagingData -> /* TODO: refactor reports to use the isShowingContent / isExpanded / isCollapsed attributes from StatusViewData instead of StatusViewState */ - pagingData.map { status -> status.toViewData(false, false, false) } + pagingData.map { status -> StatusViewData.from(status, false, false, false) } } .cachedIn(viewModelScope) diff --git a/app/src/main/java/app/pachli/components/report/adapter/StatusViewHolder.kt b/app/src/main/java/app/pachli/components/report/adapter/StatusViewHolder.kt index a6f75e678..8fa2fa5ee 100644 --- a/app/src/main/java/app/pachli/components/report/adapter/StatusViewHolder.kt +++ b/app/src/main/java/app/pachli/components/report/adapter/StatusViewHolder.kt @@ -38,8 +38,8 @@ import app.pachli.util.setClickableMentions import app.pachli.util.setClickableText import app.pachli.util.shouldTrimStatus import app.pachli.util.show +import app.pachli.viewdata.PollViewData import app.pachli.viewdata.StatusViewData -import app.pachli.viewdata.toViewData import java.util.Date class StatusViewHolder( @@ -93,7 +93,10 @@ class StatusViewHolder( mediaViewHeight, ) - statusViewHelper.setupPollReadonly(viewData.status.poll.toViewData(), viewData.status.emojis, statusDisplayOptions) + viewData.status.poll?.let { + statusViewHelper.setupPollReadonly(PollViewData.from(it), viewData.status.emojis, statusDisplayOptions) + } ?: statusViewHelper.hidePoll() + setCreatedAt(viewData.status.createdAt) } diff --git a/app/src/main/java/app/pachli/components/search/SearchViewModel.kt b/app/src/main/java/app/pachli/components/search/SearchViewModel.kt index fb0ab575e..491e7651b 100644 --- a/app/src/main/java/app/pachli/components/search/SearchViewModel.kt +++ b/app/src/main/java/app/pachli/components/search/SearchViewModel.kt @@ -28,7 +28,6 @@ import app.pachli.entity.DeletedStatus import app.pachli.entity.Status import app.pachli.network.MastodonApi import app.pachli.usecase.TimelineCases -import app.pachli.util.toViewData import app.pachli.viewdata.StatusViewData import at.connyduck.calladapter.networkresult.NetworkResult import at.connyduck.calladapter.networkresult.fold @@ -58,7 +57,8 @@ class SearchViewModel @Inject constructor( private val statusesPagingSourceFactory = SearchPagingSourceFactory(mastodonApi, SearchType.Status, loadedStatuses) { it.statuses.map { status -> - status.toViewData( + StatusViewData.from( + status, isShowingContent = alwaysShowSensitiveMedia || !status.actionableStatus.sensitive, isExpanded = alwaysOpenSpoiler, isCollapsed = true, diff --git a/app/src/main/java/app/pachli/components/timeline/TimelineTypeMappers.kt b/app/src/main/java/app/pachli/components/timeline/TimelineTypeMappers.kt deleted file mode 100644 index 3276b4f15..000000000 --- a/app/src/main/java/app/pachli/components/timeline/TimelineTypeMappers.kt +++ /dev/null @@ -1,235 +0,0 @@ -/* Copyright 2021 Tusky Contributors - * - * This file is a part of Pachli. - * - * This program is free software; you can redistribute it and/or modify it under the terms of the - * GNU General Public License as published by the Free Software Foundation; either version 3 of the - * License, or (at your option) any later version. - * - * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even - * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General - * Public License for more details. - * - * You should have received a copy of the GNU General Public License along with Tusky; if not, - * see . */ - -package app.pachli.components.timeline - -import app.pachli.db.TimelineAccountEntity -import app.pachli.db.TimelineStatusEntity -import app.pachli.db.TimelineStatusWithAccount -import app.pachli.entity.Attachment -import app.pachli.entity.Card -import app.pachli.entity.Emoji -import app.pachli.entity.HashTag -import app.pachli.entity.Poll -import app.pachli.entity.Status -import app.pachli.entity.TimelineAccount -import app.pachli.viewdata.StatusViewData -import com.google.gson.Gson -import com.google.gson.reflect.TypeToken -import java.util.Date - -@Suppress("unused") -private const val TAG = "TimelineTypeMappers" - -private val attachmentArrayListType = object : TypeToken>() {}.type -private val emojisListType = object : TypeToken>() {}.type -private val mentionListType = object : TypeToken>() {}.type -private val tagListType = object : TypeToken>() {}.type - -fun TimelineAccount.toEntity(accountId: Long, gson: Gson): TimelineAccountEntity { - return TimelineAccountEntity( - serverId = id, - timelineUserId = accountId, - localUsername = localUsername, - username = username, - displayName = name, - url = url, - avatar = avatar, - emojis = gson.toJson(emojis), - bot = bot, - ) -} - -fun TimelineAccountEntity.toAccount(gson: Gson): TimelineAccount { - return TimelineAccount( - id = serverId, - localUsername = localUsername, - username = username, - displayName = displayName, - note = "", - url = url, - avatar = avatar, - bot = bot, - emojis = gson.fromJson(emojis, emojisListType), - ) -} - -fun Status.toEntity( - timelineUserId: Long, - gson: Gson, -): TimelineStatusEntity { - return TimelineStatusEntity( - serverId = this.id, - url = actionableStatus.url, - timelineUserId = timelineUserId, - authorServerId = actionableStatus.account.id, - inReplyToId = actionableStatus.inReplyToId, - inReplyToAccountId = actionableStatus.inReplyToAccountId, - content = actionableStatus.content, - createdAt = actionableStatus.createdAt.time, - editedAt = actionableStatus.editedAt?.time, - emojis = actionableStatus.emojis.let(gson::toJson), - reblogsCount = actionableStatus.reblogsCount, - favouritesCount = actionableStatus.favouritesCount, - reblogged = actionableStatus.reblogged, - favourited = actionableStatus.favourited, - bookmarked = actionableStatus.bookmarked, - sensitive = actionableStatus.sensitive, - spoilerText = actionableStatus.spoilerText, - visibility = actionableStatus.visibility, - attachments = actionableStatus.attachments.let(gson::toJson), - mentions = actionableStatus.mentions.let(gson::toJson), - tags = actionableStatus.tags.let(gson::toJson), - application = actionableStatus.application.let(gson::toJson), - reblogServerId = reblog?.id, - reblogAccountId = reblog?.let { this.account.id }, - poll = actionableStatus.poll.let(gson::toJson), - muted = actionableStatus.muted, - pinned = actionableStatus.pinned == true, - card = actionableStatus.card?.let(gson::toJson), - repliesCount = actionableStatus.repliesCount, - language = actionableStatus.language, - filtered = actionableStatus.filtered, - ) -} - -fun TimelineStatusWithAccount.toViewData(gson: Gson, alwaysOpenSpoiler: Boolean, alwaysShowSensitiveMedia: Boolean, isDetailed: Boolean = false): StatusViewData { - val attachments: ArrayList = gson.fromJson( - status.attachments, - attachmentArrayListType, - ) ?: arrayListOf() - val mentions: List = gson.fromJson( - status.mentions, - mentionListType, - ) ?: emptyList() - val tags: List? = gson.fromJson( - status.tags, - tagListType, - ) - val application = gson.fromJson(status.application, Status.Application::class.java) - val emojis: List = gson.fromJson( - status.emojis, - emojisListType, - ) ?: emptyList() - val poll: Poll? = gson.fromJson(status.poll, Poll::class.java) - val card: Card? = gson.fromJson(status.card, Card::class.java) - - val reblog = status.reblogServerId?.let { id -> - Status( - id = id, - url = status.url, - account = account.toAccount(gson), - inReplyToId = status.inReplyToId, - inReplyToAccountId = status.inReplyToAccountId, - reblog = null, - content = status.content.orEmpty(), - createdAt = Date(status.createdAt), - editedAt = status.editedAt?.let { Date(it) }, - emojis = emojis, - reblogsCount = status.reblogsCount, - favouritesCount = status.favouritesCount, - reblogged = status.reblogged, - favourited = status.favourited, - bookmarked = status.bookmarked, - sensitive = status.sensitive, - spoilerText = status.spoilerText, - visibility = status.visibility, - attachments = attachments, - mentions = mentions, - tags = tags, - application = application, - pinned = false, - muted = status.muted, - poll = poll, - card = card, - repliesCount = status.repliesCount, - language = status.language, - filtered = status.filtered, - ) - } - val status = if (reblog != null) { - Status( - id = status.serverId, - url = null, // no url for reblogs - account = this.reblogAccount!!.toAccount(gson), - inReplyToId = null, - inReplyToAccountId = null, - reblog = reblog, - content = "", - createdAt = Date(status.createdAt), // lie but whatever? - editedAt = null, - emojis = listOf(), - reblogsCount = 0, - favouritesCount = 0, - reblogged = false, - favourited = false, - bookmarked = false, - sensitive = false, - spoilerText = "", - visibility = status.visibility, - attachments = ArrayList(), - mentions = listOf(), - tags = listOf(), - application = null, - pinned = status.pinned, - muted = status.muted, - poll = null, - card = null, - repliesCount = status.repliesCount, - language = status.language, - filtered = status.filtered, - ) - } else { - Status( - id = status.serverId, - url = status.url, - account = account.toAccount(gson), - inReplyToId = status.inReplyToId, - inReplyToAccountId = status.inReplyToAccountId, - reblog = null, - content = status.content.orEmpty(), - createdAt = Date(status.createdAt), - editedAt = status.editedAt?.let { Date(it) }, - emojis = emojis, - reblogsCount = status.reblogsCount, - favouritesCount = status.favouritesCount, - reblogged = status.reblogged, - favourited = status.favourited, - bookmarked = status.bookmarked, - sensitive = status.sensitive, - spoilerText = status.spoilerText, - visibility = status.visibility, - attachments = attachments, - mentions = mentions, - tags = tags, - application = application, - pinned = status.pinned, - muted = status.muted, - poll = poll, - card = card, - repliesCount = status.repliesCount, - language = status.language, - filtered = status.filtered, - ) - } - - return StatusViewData( - status = status, - isExpanded = this.viewData?.expanded ?: alwaysOpenSpoiler, - isShowingContent = this.viewData?.contentShowing ?: (alwaysShowSensitiveMedia || !status.actionableStatus.sensitive), - isCollapsed = this.viewData?.contentCollapsed ?: true, - isDetailed = isDetailed, - ) -} diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt index 5c1c87926..712dea39a 100644 --- a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt +++ b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt @@ -26,11 +26,12 @@ import androidx.paging.PagingState import androidx.paging.RemoteMediator import androidx.room.Transaction import androidx.room.withTransaction -import app.pachli.components.timeline.toEntity import app.pachli.db.AccountManager import app.pachli.db.AppDatabase import app.pachli.db.RemoteKeyEntity import app.pachli.db.RemoteKeyKind +import app.pachli.db.TimelineAccountEntity +import app.pachli.db.TimelineStatusEntity import app.pachli.db.TimelineStatusWithAccount import app.pachli.entity.Status import app.pachli.network.Links @@ -258,13 +259,15 @@ class CachedTimelineRemoteMediator( @Transaction private suspend fun insertStatuses(statuses: List) { for (status in statuses) { - timelineDao.insertAccount(status.account.toEntity(activeAccount.id, gson)) - status.reblog?.account?.toEntity(activeAccount.id, gson)?.let { rebloggedAccount -> - timelineDao.insertAccount(rebloggedAccount) + timelineDao.insertAccount(TimelineAccountEntity.from(status.account, activeAccount.id, gson)) + status.reblog?.account?.let { + val account = TimelineAccountEntity.from(it, activeAccount.id, gson) + timelineDao.insertAccount(account) } timelineDao.insertStatus( - status.toEntity( + TimelineStatusEntity.from( + status, timelineUserId = activeAccount.id, gson = gson, ), diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt index 448f0cb76..8d61ee1af 100644 --- a/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt +++ b/app/src/main/java/app/pachli/components/timeline/viewmodel/CachedTimelineViewModel.kt @@ -32,7 +32,6 @@ import app.pachli.appstore.ReblogEvent import app.pachli.components.timeline.CachedTimelineRepository import app.pachli.components.timeline.FiltersRepository import app.pachli.components.timeline.TimelineKind -import app.pachli.components.timeline.toViewData import app.pachli.db.AccountManager import app.pachli.entity.Filter import app.pachli.entity.Poll @@ -97,10 +96,11 @@ class CachedTimelineViewModel @Inject constructor( .map { pagingData -> pagingData .map { - it.toViewData( + StatusViewData.from( + it, gson, - alwaysOpenSpoiler = activeAccount.alwaysOpenSpoiler, - alwaysShowSensitiveMedia = activeAccount.alwaysShowSensitiveMedia, + isExpanded = activeAccount.alwaysOpenSpoiler, + isShowingContent = activeAccount.alwaysShowSensitiveMedia, ) } .filter { shouldFilterStatus(it) != Filter.Action.HIDE } diff --git a/app/src/main/java/app/pachli/components/timeline/viewmodel/NetworkTimelineViewModel.kt b/app/src/main/java/app/pachli/components/timeline/viewmodel/NetworkTimelineViewModel.kt index be761c559..231285b9b 100644 --- a/app/src/main/java/app/pachli/components/timeline/viewmodel/NetworkTimelineViewModel.kt +++ b/app/src/main/java/app/pachli/components/timeline/viewmodel/NetworkTimelineViewModel.kt @@ -38,7 +38,6 @@ import app.pachli.entity.Poll import app.pachli.network.FilterModel import app.pachli.settings.AccountPreferenceDataStore import app.pachli.usecase.TimelineCases -import app.pachli.util.toViewData import app.pachli.viewdata.StatusViewData import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow @@ -90,7 +89,8 @@ class NetworkTimelineViewModel @Inject constructor( return repository.getStatusStream(viewModelScope, kind = kind, initialKey = initialKey) .map { pagingData -> pagingData.map { - modifiedViewData[it.id] ?: it.toViewData( + modifiedViewData[it.id] ?: StatusViewData.from( + it, isShowingContent = statusDisplayOptions.value.showSensitiveMedia || !it.actionableStatus.sensitive, isExpanded = statusDisplayOptions.value.openSpoiler, isCollapsed = true, diff --git a/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt b/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt index e3f9cc5ca..c0d91b274 100644 --- a/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt +++ b/app/src/main/java/app/pachli/components/trending/viewmodel/TrendingTagsViewModel.kt @@ -21,10 +21,10 @@ import androidx.lifecycle.viewModelScope import app.pachli.appstore.EventHub import app.pachli.appstore.PreferenceChangedEvent import app.pachli.entity.Filter +import app.pachli.entity.TrendingTag import app.pachli.entity.end import app.pachli.entity.start import app.pachli.network.MastodonApi -import app.pachli.util.toViewData import app.pachli.viewdata.TrendingViewData import at.connyduck.calladapter.networkresult.fold import kotlinx.coroutines.async @@ -97,7 +97,7 @@ class TrendingTagsViewModel @Inject constructor( } ?: false } .sortedByDescending { tag -> tag.history.sumOf { it.uses.toLongOrNull() ?: 0 } } - .toViewData() + .toTrendingViewDataTag() val header = TrendingViewData.Header(firstTag.start(), firstTag.end()) TrendingTagsUiState(listOf(header) + tags, LoadingState.LOADED) @@ -114,6 +114,14 @@ class TrendingTagsViewModel @Inject constructor( ) } + private fun List.toTrendingViewDataTag(): List { + val maxTrendingValue = flatMap { tag -> tag.history } + .mapNotNull { it.uses.toLongOrNull() } + .maxOrNull() ?: 1 + + return map { TrendingViewData.Tag.from(it, maxTrendingValue) } + } + companion object { private const val TAG = "TrendingViewModel" } diff --git a/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt b/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt index 3f72b502d..b2fb93dc5 100644 --- a/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt +++ b/app/src/main/java/app/pachli/components/viewthread/ViewThreadViewModel.kt @@ -28,19 +28,16 @@ import app.pachli.appstore.StatusComposedEvent import app.pachli.appstore.StatusDeletedEvent import app.pachli.appstore.StatusEditedEvent import app.pachli.components.timeline.CachedTimelineRepository -import app.pachli.components.timeline.toViewData import app.pachli.components.timeline.util.ifExpected import app.pachli.db.AccountEntity import app.pachli.db.AccountManager import app.pachli.db.AppDatabase -import app.pachli.db.StatusViewDataEntity import app.pachli.entity.Filter import app.pachli.entity.FilterV1 import app.pachli.entity.Status import app.pachli.network.FilterModel import app.pachli.network.MastodonApi import app.pachli.usecase.TimelineCases -import app.pachli.util.toViewData import app.pachli.viewdata.StatusViewData import at.connyduck.calladapter.networkresult.fold import at.connyduck.calladapter.networkresult.getOrElse @@ -113,25 +110,32 @@ class ViewThreadViewModel @Inject constructor( viewModelScope.launch { Log.d(TAG, "Finding status with: $id") val contextCall = async { api.statusContext(id) } - val timelineStatus = db.timelineDao().getStatus(id) + val timelineStatusWithAccount = db.timelineDao().getStatus(id) - var detailedStatus = if (timelineStatus != null) { + var detailedStatus = if (timelineStatusWithAccount != null) { Log.d(TAG, "Loaded status from local timeline") - val viewData = timelineStatus.toViewData( - gson, - alwaysOpenSpoiler = alwaysOpenSpoiler, - alwaysShowSensitiveMedia = alwaysShowSensitiveMedia, - isDetailed = true, - ) + val status = timelineStatusWithAccount.toStatus(gson) // Return the correct status, depending on which one matched. If you do not do // this the status IDs will be different between the status that's displayed with // ThreadUiState.LoadingThread and ThreadUiState.Success, even though the apparent // status content is the same. Then the status flickers as it is drawn twice. - if (viewData.actionableId == id) { - viewData.actionable.toViewData(isDetailed = true, viewData) + if (status.actionableId == id) { + StatusViewData.from( + status = status.actionableStatus, + isExpanded = timelineStatusWithAccount.viewData?.expanded ?: alwaysOpenSpoiler, + isShowingContent = timelineStatusWithAccount.viewData?.contentShowing ?: alwaysShowSensitiveMedia, + isCollapsed = timelineStatusWithAccount.viewData?.contentCollapsed ?: true, + isDetailed = true, + ) } else { - viewData + StatusViewData.from( + timelineStatusWithAccount, + gson, + isExpanded = alwaysOpenSpoiler, + isShowingContent = alwaysShowSensitiveMedia, + isDetailed = true, + ) } } else { Log.d(TAG, "Loaded status from network") @@ -139,7 +143,7 @@ class ViewThreadViewModel @Inject constructor( _uiState.value = ThreadUiState.Error(exception) return@launch } - result.toViewData(isDetailed = true) + StatusViewData.fromStatusAndUiState(result, isDetailed = true) } _uiState.value = ThreadUiState.LoadingThread( @@ -151,9 +155,16 @@ class ViewThreadViewModel @Inject constructor( // compared to the remote one. Now the user has a working UI do a background fetch // for the status. Ignore errors, the user still has a functioning UI if the fetch // failed. - if (timelineStatus != null) { - val viewData = api.status(id).getOrNull()?.toViewData(isDetailed = true, detailedStatus) - if (viewData != null) { detailedStatus = viewData } + if (timelineStatusWithAccount != null) { + api.status(id).getOrNull()?.let { + detailedStatus = StatusViewData.from( + it, + isShowingContent = detailedStatus.isShowingContent, + isExpanded = detailedStatus.isExpanded, + isCollapsed = detailedStatus.isCollapsed, + isDetailed = true, + ) + } } val contextResult = contextCall.await() @@ -163,12 +174,26 @@ class ViewThreadViewModel @Inject constructor( val cachedViewData = repository.getStatusViewData(ids) val ancestors = statusContext.ancestors.map { status -> - status.toViewData(statusViewDataEntity = cachedViewData[status.id]) - }.filter() + val svd = cachedViewData[status.id] + StatusViewData.from( + status, + isShowingContent = svd?.contentShowing ?: (alwaysShowSensitiveMedia || !status.actionableStatus.sensitive), + isExpanded = svd?.expanded ?: alwaysOpenSpoiler, + isCollapsed = svd?.contentCollapsed ?: true, + isDetailed = false, + ) + }.filterByFilterAction() val descendants = statusContext.descendants.map { status -> - status.toViewData(statusViewDataEntity = cachedViewData[status.id]) - }.filter() + val svd = cachedViewData[status.id] + StatusViewData.from( + status, + isShowingContent = svd?.contentShowing ?: (alwaysShowSensitiveMedia || !status.actionableStatus.sensitive), + isExpanded = svd?.expanded ?: alwaysOpenSpoiler, + isCollapsed = svd?.contentCollapsed ?: true, + isDetailed = false, + ) + }.filterByFilterAction() val statuses = ancestors + detailedStatus + descendants _uiState.value = ThreadUiState.Success( @@ -345,7 +370,7 @@ class ViewThreadViewModel @Inject constructor( if (detailedIndex != -1 && repliedIndex >= detailedIndex) { // there is a new reply to the detailed status or below -> display it val newStatuses = statuses.subList(0, repliedIndex + 1) + - eventStatus.toViewData() + + StatusViewData.fromStatusAndUiState(eventStatus) + statuses.subList(repliedIndex + 1, statuses.size) uiState.copy(statusViewData = newStatuses) } else { @@ -359,7 +384,7 @@ class ViewThreadViewModel @Inject constructor( uiState.copy( statusViewData = uiState.statusViewData.map { status -> if (status.actionableId == event.originalId) { - event.status.toViewData() + StatusViewData.fromStatusAndUiState(event.status) } else { status } @@ -465,7 +490,7 @@ class ViewThreadViewModel @Inject constructor( private fun updateStatuses() { updateSuccess { uiState -> - val statuses = uiState.statusViewData.filter() + val statuses = uiState.statusViewData.filterByFilterAction() uiState.copy( statusViewData = statuses, revealButton = statuses.getRevealButtonState(), @@ -473,7 +498,7 @@ class ViewThreadViewModel @Inject constructor( } } - private fun List.filter(): List { + private fun List.filterByFilterAction(): List { return filter { status -> if (status.isDetailed) { true @@ -485,35 +510,14 @@ class ViewThreadViewModel @Inject constructor( } /** - * Convert the status to a [StatusViewData], copying the view data from [statusViewData] + * Creates a [StatusViewData] from `status`, copying over the viewdata state from the same + * status in _uiState (if that status exists). */ - private fun Status.toViewData(isDetailed: Boolean = false, statusViewData: StatusViewData): StatusViewData { - return toViewData( - isShowingContent = statusViewData.isShowingContent, - isExpanded = statusViewData.isExpanded, - isCollapsed = statusViewData.isCollapsed, - isDetailed = isDetailed, - ) - } - - /** - * Convert the status to a [StatusViewData], copying the view data from [statusViewDataEntity] - */ - private fun Status.toViewData(isDetailed: Boolean = false, statusViewDataEntity: StatusViewDataEntity?): StatusViewData { - return toViewData( - isShowingContent = statusViewDataEntity?.contentShowing ?: (alwaysShowSensitiveMedia || !actionableStatus.sensitive), - isExpanded = statusViewDataEntity?.expanded ?: alwaysOpenSpoiler, - isCollapsed = statusViewDataEntity?.contentCollapsed ?: !isDetailed, - isDetailed = isDetailed, - ) - } - - private fun Status.toViewData( - isDetailed: Boolean = false, - ): StatusViewData { - val oldStatus = (_uiState.value as? ThreadUiState.Success)?.statusViewData?.find { it.id == this.id } - return toViewData( - isShowingContent = oldStatus?.isShowingContent ?: (alwaysShowSensitiveMedia || !actionableStatus.sensitive), + private fun StatusViewData.Companion.fromStatusAndUiState(status: Status, isDetailed: Boolean = false): StatusViewData { + val oldStatus = (_uiState.value as? ThreadUiState.Success)?.statusViewData?.find { it.id == status.id } + return from( + status, + isShowingContent = oldStatus?.isShowingContent ?: (alwaysShowSensitiveMedia || !status.actionableStatus.sensitive), isExpanded = oldStatus?.isExpanded ?: alwaysOpenSpoiler, isCollapsed = oldStatus?.isCollapsed ?: !isDetailed, isDetailed = oldStatus?.isDetailed ?: isDetailed, diff --git a/app/src/main/java/app/pachli/components/viewthread/edits/ViewEditsAdapter.kt b/app/src/main/java/app/pachli/components/viewthread/edits/ViewEditsAdapter.kt index 3e077ab2f..0bc6d7156 100644 --- a/app/src/main/java/app/pachli/components/viewthread/edits/ViewEditsAdapter.kt +++ b/app/src/main/java/app/pachli/components/viewthread/edits/ViewEditsAdapter.kt @@ -35,7 +35,7 @@ import app.pachli.util.parseAsMastodonHtml import app.pachli.util.setClickableText import app.pachli.util.show import app.pachli.util.visible -import app.pachli.viewdata.toViewData +import app.pachli.viewdata.PollOptionViewData import com.bumptech.glide.Glide import com.google.android.material.color.MaterialColors import org.xml.sax.XMLReader @@ -138,7 +138,7 @@ class ViewEditsAdapter( binding.statusEditPollOptions.layoutManager = LinearLayoutManager(context) pollAdapter.setup( - options = edit.poll.options.map { it.toViewData(false) }, + options = edit.poll.options.map { PollOptionViewData.from(it, false) }, voteCount = 0, votersCount = null, emojis = edit.emojis, diff --git a/app/src/main/java/app/pachli/db/TimelineStatusEntity.kt b/app/src/main/java/app/pachli/db/TimelineStatusEntity.kt index 6feb5e3b8..c1da12d14 100644 --- a/app/src/main/java/app/pachli/db/TimelineStatusEntity.kt +++ b/app/src/main/java/app/pachli/db/TimelineStatusEntity.kt @@ -20,8 +20,18 @@ import androidx.room.Entity import androidx.room.ForeignKey import androidx.room.Index import androidx.room.TypeConverters +import app.pachli.entity.Attachment +import app.pachli.entity.Card +import app.pachli.entity.Emoji import app.pachli.entity.FilterResult +import app.pachli.entity.HashTag +import app.pachli.entity.Poll import app.pachli.entity.Status +import app.pachli.entity.TimelineAccount +import com.google.gson.Gson +import com.google.gson.reflect.TypeToken +import java.lang.reflect.Type +import java.util.Date /** * We're trying to play smart here. Server sends us reblogs as two entities one embedded into @@ -82,7 +92,43 @@ data class TimelineStatusEntity( val card: String?, val language: String?, val filtered: List?, -) +) { + companion object { + fun from(status: Status, timelineUserId: Long, gson: Gson) = TimelineStatusEntity( + serverId = status.id, + url = status.actionableStatus.url, + timelineUserId = timelineUserId, + authorServerId = status.actionableStatus.account.id, + inReplyToId = status.actionableStatus.inReplyToId, + inReplyToAccountId = status.actionableStatus.inReplyToAccountId, + content = status.actionableStatus.content, + createdAt = status.actionableStatus.createdAt.time, + editedAt = status.actionableStatus.editedAt?.time, + emojis = status.actionableStatus.emojis.let(gson::toJson), + reblogsCount = status.actionableStatus.reblogsCount, + favouritesCount = status.actionableStatus.favouritesCount, + reblogged = status.actionableStatus.reblogged, + favourited = status.actionableStatus.favourited, + bookmarked = status.actionableStatus.bookmarked, + sensitive = status.actionableStatus.sensitive, + spoilerText = status.actionableStatus.spoilerText, + visibility = status.actionableStatus.visibility, + attachments = status.actionableStatus.attachments.let(gson::toJson), + mentions = status.actionableStatus.mentions.let(gson::toJson), + tags = status.actionableStatus.tags.let(gson::toJson), + application = status.actionableStatus.application.let(gson::toJson), + reblogServerId = status.reblog?.id, + reblogAccountId = status.reblog?.let { status.account.id }, + poll = status.actionableStatus.poll.let(gson::toJson), + muted = status.actionableStatus.muted, + pinned = status.actionableStatus.pinned == true, + card = status.actionableStatus.card?.let(gson::toJson), + repliesCount = status.actionableStatus.repliesCount, + language = status.actionableStatus.language, + filtered = status.actionableStatus.filtered, + ) + } +} @Entity( primaryKeys = ["serverId", "timelineUserId"], @@ -97,7 +143,35 @@ data class TimelineAccountEntity( val avatar: String, val emojis: String, val bot: Boolean, -) +) { + fun toTimelineAccount(gson: Gson): TimelineAccount { + return TimelineAccount( + id = serverId, + localUsername = localUsername, + username = username, + displayName = displayName, + note = "", + url = url, + avatar = avatar, + bot = bot, + emojis = gson.fromJson(emojis, emojisListType), + ) + } + + companion object { + fun from(timelineAccount: TimelineAccount, accountId: Long, gson: Gson) = TimelineAccountEntity( + serverId = timelineAccount.id, + timelineUserId = accountId, + localUsername = timelineAccount.localUsername, + username = timelineAccount.username, + displayName = timelineAccount.name, + url = timelineAccount.url, + avatar = timelineAccount.avatar, + emojis = gson.toJson(timelineAccount.emojis), + bot = timelineAccount.bot, + ) + } +} /** * The local view data for a status. @@ -120,6 +194,11 @@ data class StatusViewDataEntity( val contentCollapsed: Boolean, ) +val attachmentArrayListType: Type = object : TypeToken>() {}.type +val emojisListType: Type = object : TypeToken>() {}.type +val mentionListType: Type = object : TypeToken>() {}.type +val tagListType: Type = object : TypeToken>() {}.type + data class TimelineStatusWithAccount( @Embedded val status: TimelineStatusEntity, @@ -129,4 +208,125 @@ data class TimelineStatusWithAccount( val reblogAccount: TimelineAccountEntity? = null, // null when no reblog @Embedded(prefix = "svd_") val viewData: StatusViewDataEntity? = null, -) +) { + fun toStatus(gson: Gson): Status { + val attachments: ArrayList = gson.fromJson( + status.attachments, + attachmentArrayListType, + ) ?: arrayListOf() + val mentions: List = gson.fromJson( + status.mentions, + mentionListType, + ) ?: emptyList() + val tags: List? = gson.fromJson( + status.tags, + tagListType, + ) + val application = gson.fromJson(status.application, Status.Application::class.java) + val emojis: List = gson.fromJson( + status.emojis, + emojisListType, + ) ?: emptyList() + val poll: Poll? = gson.fromJson(status.poll, Poll::class.java) + val card: Card? = gson.fromJson(status.card, Card::class.java) + + val reblog = status.reblogServerId?.let { id -> + Status( + id = id, + url = status.url, + account = account.toTimelineAccount(gson), + inReplyToId = status.inReplyToId, + inReplyToAccountId = status.inReplyToAccountId, + reblog = null, + content = status.content.orEmpty(), + createdAt = Date(status.createdAt), + editedAt = status.editedAt?.let { Date(it) }, + emojis = emojis, + reblogsCount = status.reblogsCount, + favouritesCount = status.favouritesCount, + reblogged = status.reblogged, + favourited = status.favourited, + bookmarked = status.bookmarked, + sensitive = status.sensitive, + spoilerText = status.spoilerText, + visibility = status.visibility, + attachments = attachments, + mentions = mentions, + tags = tags, + application = application, + pinned = false, + muted = status.muted, + poll = poll, + card = card, + repliesCount = status.repliesCount, + language = status.language, + filtered = status.filtered, + ) + } + return if (reblog != null) { + Status( + id = status.serverId, + url = null, // no url for reblogs + account = reblogAccount!!.toTimelineAccount(gson), + inReplyToId = null, + inReplyToAccountId = null, + reblog = reblog, + content = "", + createdAt = Date(status.createdAt), // lie but whatever? + editedAt = null, + emojis = listOf(), + reblogsCount = 0, + favouritesCount = 0, + reblogged = false, + favourited = false, + bookmarked = false, + sensitive = false, + spoilerText = "", + visibility = status.visibility, + attachments = ArrayList(), + mentions = listOf(), + tags = listOf(), + application = null, + pinned = status.pinned, + muted = status.muted, + poll = null, + card = null, + repliesCount = status.repliesCount, + language = status.language, + filtered = status.filtered, + ) + } else { + Status( + id = status.serverId, + url = status.url, + account = account.toTimelineAccount(gson), + inReplyToId = status.inReplyToId, + inReplyToAccountId = status.inReplyToAccountId, + reblog = null, + content = status.content.orEmpty(), + createdAt = Date(status.createdAt), + editedAt = status.editedAt?.let { Date(it) }, + emojis = emojis, + reblogsCount = status.reblogsCount, + favouritesCount = status.favouritesCount, + reblogged = status.reblogged, + favourited = status.favourited, + bookmarked = status.bookmarked, + sensitive = status.sensitive, + spoilerText = status.spoilerText, + visibility = status.visibility, + attachments = attachments, + mentions = mentions, + tags = tags, + application = application, + pinned = status.pinned, + muted = status.muted, + poll = poll, + card = card, + repliesCount = status.repliesCount, + language = status.language, + filtered = status.filtered, + ) + } + } +} diff --git a/app/src/main/java/app/pachli/util/StatusViewHelper.kt b/app/src/main/java/app/pachli/util/StatusViewHelper.kt index 146c54bc4..f5b35e58a 100644 --- a/app/src/main/java/app/pachli/util/StatusViewHelper.kt +++ b/app/src/main/java/app/pachli/util/StatusViewHelper.kt @@ -261,7 +261,10 @@ class StatusViewHelper(private val itemView: View) { } } - fun setupPollReadonly(poll: PollViewData?, emojis: List, statusDisplayOptions: StatusDisplayOptions) { + /** + * Configures and shows poll views based on [poll]. + */ + fun setupPollReadonly(poll: PollViewData, emojis: List, statusDisplayOptions: StatusDisplayOptions) { val pollResults = listOf( itemView.findViewById(R.id.status_poll_option_result_0), itemView.findViewById(R.id.status_poll_option_result_1), @@ -271,19 +274,29 @@ class StatusViewHelper(private val itemView: View) { val pollDescription = itemView.findViewById(R.id.status_poll_description) - if (poll == null) { - for (pollResult in pollResults) { - pollResult.visibility = View.GONE - } - pollDescription.visibility = View.GONE - } else { - val timestamp = System.currentTimeMillis() + val timestamp = System.currentTimeMillis() - setupPollResult(poll, emojis, pollResults, statusDisplayOptions.animateEmojis) + setupPollResult(poll, emojis, pollResults, statusDisplayOptions.animateEmojis) - pollDescription.visibility = View.VISIBLE - pollDescription.text = getPollInfoText(timestamp, poll, pollDescription, statusDisplayOptions.useAbsoluteTime) + pollDescription.show() + pollDescription.text = getPollInfoText(timestamp, poll, pollDescription, statusDisplayOptions.useAbsoluteTime) + } + + /** + * Hides views related to polls. + */ + fun hidePoll() { + val pollResults = listOf( + itemView.findViewById(R.id.status_poll_option_result_0), + itemView.findViewById(R.id.status_poll_option_result_1), + itemView.findViewById(R.id.status_poll_option_result_2), + itemView.findViewById(R.id.status_poll_option_result_3), + ) + + for (pollResult in pollResults) { + pollResult.hide() } + itemView.findViewById(R.id.status_poll_description).hide() } private fun getPollInfoText(timestamp: Long, poll: PollViewData, pollDescription: TextView, useAbsoluteTime: Boolean): CharSequence { diff --git a/app/src/main/java/app/pachli/util/ViewDataUtils.kt b/app/src/main/java/app/pachli/util/ViewDataUtils.kt deleted file mode 100644 index a2654c5af..000000000 --- a/app/src/main/java/app/pachli/util/ViewDataUtils.kt +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Copyright 2023 Tusky Contributors - * - * This file is a part of Pachli. - * - * This program is free software; you can redistribute it and/or modify it under the terms of the - * GNU General Public License as published by the Free Software Foundation; either version 3 of the - * License, or (at your option) any later version. - * - * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even - * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General - * Public License for more details. - * - * You should have received a copy of the GNU General Public License along with Tusky; if not, - * see . - */ - -@file:JvmName("ViewDataUtils") - -/* Copyright 2017 Andrew Dawson - * - * This file is a part of Pachli. - * - * This program is free software; you can redistribute it and/or modify it under the terms of the - * GNU General Public License as published by the Free Software Foundation; either version 3 of the - * License, or (at your option) any later version. - * - * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even - * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General - * Public License for more details. - * - * You should have received a copy of the GNU General Public License along with Tusky; if not, - * see . */ -package app.pachli.util - -import app.pachli.entity.Filter -import app.pachli.entity.Notification -import app.pachli.entity.Status -import app.pachli.entity.TrendingTag -import app.pachli.viewdata.NotificationViewData -import app.pachli.viewdata.StatusViewData -import app.pachli.viewdata.TrendingViewData - -fun Status.toViewData( - isShowingContent: Boolean, - isExpanded: Boolean, - isCollapsed: Boolean, - isDetailed: Boolean = false, - filterAction: Filter.Action = app.pachli.entity.Filter.Action.NONE, -): StatusViewData { - return StatusViewData( - status = this, - isShowingContent = isShowingContent, - isCollapsed = isCollapsed, - isExpanded = isExpanded, - isDetailed = isDetailed, - filterAction = filterAction, - ) -} - -fun Notification.toViewData( - isShowingContent: Boolean, - isExpanded: Boolean, - isCollapsed: Boolean, - filterAction: Filter.Action, -): NotificationViewData { - return NotificationViewData( - this.type, - this.id, - this.account, - this.status?.toViewData( - isShowingContent, - isExpanded, - isCollapsed, - filterAction = filterAction, - ), - this.report, - ) -} - -fun List.toViewData(): List { - val maxTrendingValue = flatMap { tag -> tag.history } - .mapNotNull { it.uses.toLongOrNull() } - .maxOrNull() ?: 1 - - return map { tag -> - - val reversedHistory = tag.history.asReversed() - - TrendingViewData.Tag( - name = tag.name, - usage = reversedHistory.mapNotNull { it.uses.toLongOrNull() }, - accounts = reversedHistory.mapNotNull { it.accounts.toLongOrNull() }, - maxTrendingValue = maxTrendingValue, - ) - } -} diff --git a/app/src/main/java/app/pachli/viewdata/NotificationViewData.kt b/app/src/main/java/app/pachli/viewdata/NotificationViewData.kt index 5c1456c5b..983200ef5 100644 --- a/app/src/main/java/app/pachli/viewdata/NotificationViewData.kt +++ b/app/src/main/java/app/pachli/viewdata/NotificationViewData.kt @@ -17,6 +17,7 @@ package app.pachli.viewdata +import app.pachli.entity.Filter import app.pachli.entity.Notification import app.pachli.entity.Report import app.pachli.entity.TimelineAccount @@ -27,4 +28,28 @@ data class NotificationViewData( val account: TimelineAccount, var statusViewData: StatusViewData?, val report: Report?, -) +) { + companion object { + fun from( + notification: Notification, + isShowingContent: Boolean, + isExpanded: Boolean, + isCollapsed: Boolean, + filterAction: Filter.Action, + ) = NotificationViewData( + notification.type, + notification.id, + notification.account, + notification.status?.let { status -> + StatusViewData.from( + status, + isShowingContent, + isExpanded, + isCollapsed, + filterAction = filterAction, + ) + }, + notification.report, + ) + } +} diff --git a/app/src/main/java/app/pachli/viewdata/PollViewData.kt b/app/src/main/java/app/pachli/viewdata/PollViewData.kt index 9e8872765..00b36efbc 100644 --- a/app/src/main/java/app/pachli/viewdata/PollViewData.kt +++ b/app/src/main/java/app/pachli/viewdata/PollViewData.kt @@ -34,14 +34,36 @@ data class PollViewData( val votersCount: Int?, val options: List, var voted: Boolean, -) +) { + companion object { + fun from(poll: Poll) = PollViewData( + id = poll.id, + expiresAt = poll.expiresAt, + expired = poll.expired, + multiple = poll.multiple, + votesCount = poll.votesCount, + votersCount = poll.votersCount, + options = poll.options.mapIndexed { index, option -> PollOptionViewData.from(option, poll.ownVotes?.contains(index) == true) }, + voted = poll.voted, + ) + } +} data class PollOptionViewData( val title: String, var votesCount: Int, var selected: Boolean, var voted: Boolean, -) +) { + companion object { + fun from(pollOption: PollOption, voted: Boolean) = PollOptionViewData( + title = pollOption.title, + votesCount = pollOption.votesCount, + selected = false, + voted = voted, + ) + } +} fun calculatePercent(fraction: Int, totalVoters: Int?, totalVotes: Int): Int { return if (fraction == 0) { @@ -61,26 +83,3 @@ fun buildDescription(title: String, percent: Int, voted: Boolean, context: Conte } return builder.append(title) } - -fun Poll?.toViewData(): PollViewData? { - if (this == null) return null - return PollViewData( - id = id, - expiresAt = expiresAt, - expired = expired, - multiple = multiple, - votesCount = votesCount, - votersCount = votersCount, - options = options.mapIndexed { index, option -> option.toViewData(ownVotes?.contains(index) == true) }, - voted = voted, - ) -} - -fun PollOption.toViewData(voted: Boolean): PollOptionViewData { - return PollOptionViewData( - title = title, - votesCount = votesCount, - selected = false, - voted = voted, - ) -} diff --git a/app/src/main/java/app/pachli/viewdata/StatusViewData.kt b/app/src/main/java/app/pachli/viewdata/StatusViewData.kt index 29e92563e..3a6ab4d7d 100644 --- a/app/src/main/java/app/pachli/viewdata/StatusViewData.kt +++ b/app/src/main/java/app/pachli/viewdata/StatusViewData.kt @@ -16,11 +16,16 @@ package app.pachli.viewdata import android.os.Build import android.text.Spanned +import app.pachli.components.conversation.ConversationAccountEntity +import app.pachli.components.conversation.ConversationStatusEntity +import app.pachli.db.TimelineStatusWithAccount import app.pachli.entity.Filter +import app.pachli.entity.Poll import app.pachli.entity.Status import app.pachli.util.parseAsMastodonHtml import app.pachli.util.replaceCrashingCharacters import app.pachli.util.shouldTrimStatus +import com.google.gson.Gson /** * Data required to display a status. @@ -113,4 +118,111 @@ data class StatusViewData( /** Helper for Java */ fun copyWithCollapsed(isCollapsed: Boolean) = copy(isCollapsed = isCollapsed) + + fun toConversationStatusEntity( + favourited: Boolean = status.favourited, + bookmarked: Boolean = status.bookmarked, + muted: Boolean = status.muted ?: false, + poll: Poll? = status.poll, + expanded: Boolean = isExpanded, + collapsed: Boolean = isCollapsed, + showingHiddenContent: Boolean = isShowingContent, + ) = ConversationStatusEntity( + id = id, + url = status.url, + inReplyToId = status.inReplyToId, + inReplyToAccountId = status.inReplyToAccountId, + account = ConversationAccountEntity.from(status.account), + content = status.content, + createdAt = status.createdAt, + editedAt = status.editedAt, + emojis = status.emojis, + favouritesCount = status.favouritesCount, + repliesCount = status.repliesCount, + favourited = favourited, + bookmarked = bookmarked, + sensitive = status.sensitive, + spoilerText = status.spoilerText, + attachments = status.attachments, + mentions = status.mentions, + tags = status.tags, + showingHiddenContent = showingHiddenContent, + expanded = expanded, + collapsed = collapsed, + muted = muted, + poll = poll, + language = status.language, + ) + + companion object { + fun from( + status: Status, + isShowingContent: Boolean, + isExpanded: Boolean, + isCollapsed: Boolean, + isDetailed: Boolean = false, + filterAction: Filter.Action = Filter.Action.NONE, + ) = StatusViewData( + status = status, + isShowingContent = isShowingContent, + isCollapsed = isCollapsed, + isExpanded = isExpanded, + isDetailed = isDetailed, + filterAction = filterAction, + ) + + fun from(conversationStatusEntity: ConversationStatusEntity) = StatusViewData( + status = Status( + id = conversationStatusEntity.id, + url = conversationStatusEntity.url, + account = conversationStatusEntity.account.toAccount(), + inReplyToId = conversationStatusEntity.inReplyToId, + inReplyToAccountId = conversationStatusEntity.inReplyToAccountId, + content = conversationStatusEntity.content, + reblog = null, + createdAt = conversationStatusEntity.createdAt, + editedAt = conversationStatusEntity.editedAt, + emojis = conversationStatusEntity.emojis, + reblogsCount = 0, + favouritesCount = conversationStatusEntity.favouritesCount, + repliesCount = conversationStatusEntity.repliesCount, + reblogged = false, + favourited = conversationStatusEntity.favourited, + bookmarked = conversationStatusEntity.bookmarked, + sensitive = conversationStatusEntity.sensitive, + spoilerText = conversationStatusEntity.spoilerText, + visibility = Status.Visibility.DIRECT, + attachments = conversationStatusEntity.attachments, + mentions = conversationStatusEntity.mentions, + tags = conversationStatusEntity.tags, + application = null, + pinned = false, + muted = conversationStatusEntity.muted, + poll = conversationStatusEntity.poll, + card = null, + language = conversationStatusEntity.language, + filtered = null, + ), + isExpanded = conversationStatusEntity.expanded, + isShowingContent = conversationStatusEntity.showingHiddenContent, + isCollapsed = conversationStatusEntity.collapsed, + ) + + fun from( + timelineStatusWithAccount: TimelineStatusWithAccount, + gson: Gson, + isExpanded: Boolean, + isShowingContent: Boolean, + isDetailed: Boolean = false, + ): StatusViewData { + val status = timelineStatusWithAccount.toStatus(gson) + return StatusViewData( + status = status, + isExpanded = timelineStatusWithAccount.viewData?.expanded ?: isExpanded, + isShowingContent = timelineStatusWithAccount.viewData?.contentShowing ?: (isShowingContent || !status.actionableStatus.sensitive), + isCollapsed = timelineStatusWithAccount.viewData?.contentCollapsed ?: true, + isDetailed = isDetailed, + ) + } + } } diff --git a/app/src/main/java/app/pachli/viewdata/TrendingViewData.kt b/app/src/main/java/app/pachli/viewdata/TrendingViewData.kt index 6e316dc41..296441680 100644 --- a/app/src/main/java/app/pachli/viewdata/TrendingViewData.kt +++ b/app/src/main/java/app/pachli/viewdata/TrendingViewData.kt @@ -15,6 +15,7 @@ package app.pachli.viewdata +import app.pachli.entity.TrendingTag import java.util.Date sealed class TrendingViewData { @@ -36,5 +37,19 @@ sealed class TrendingViewData { ) : TrendingViewData() { override val id: String get() = name + + companion object { + fun from(trendingTag: TrendingTag, maxTrendingValue: Long): Tag { + // Reverse the list to put oldest items first + val reversedHistory = trendingTag.history.asReversed() + + return Tag( + name = trendingTag.name, + usage = reversedHistory.map { it.uses.toLongOrNull() ?: 0 }, + accounts = reversedHistory.map { it.accounts.toLongOrNull() ?: 0 }, + maxTrendingValue = maxTrendingValue, + ) + } + } } } diff --git a/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt b/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt index e30bd8031..05a2cdf5f 100644 --- a/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt +++ b/app/src/test/java/app/pachli/components/timeline/StatusMocker.kt @@ -1,6 +1,8 @@ package app.pachli.components.timeline import app.pachli.db.StatusViewDataEntity +import app.pachli.db.TimelineAccountEntity +import app.pachli.db.TimelineStatusEntity import app.pachli.db.TimelineStatusWithAccount import app.pachli.entity.Status import app.pachli.entity.TimelineAccount @@ -95,11 +97,13 @@ fun mockStatusEntityWithAccount( val gson = Gson() return TimelineStatusWithAccount( - status = mockedStatus.toEntity( + status = TimelineStatusEntity.from( + mockedStatus, timelineUserId = userId, gson = gson, ), - account = mockedStatus.account.toEntity( + account = TimelineAccountEntity.from( + mockedStatus.account, accountId = userId, gson = gson, ), diff --git a/docs/code-style.md b/docs/code-style.md new file mode 100644 index 000000000..9ce863a14 --- /dev/null +++ b/docs/code-style.md @@ -0,0 +1,122 @@ +# Code style guide + +## Synopsis + +This document describes aspects of code style that are not enforced with linters or formatting tools but the project still tries to adhere to. Some of these are things that developers might reasonably disagree on, but the project has a specific stance. + +## Topics + +### On converting between types + +Roughly speaking the code that handles data from the user's server deals with three variations of that data. + +1. Data from the network +2. Data cached locally (either to disk or memory) +3. Data displayed to the user + +There must be code to convert between those representations, and it's important to make sure there isn't a loop in the dependency graph between the types. + +Consider two types, `N`, representing data received from the network, and `C`, representing data that will be cached. + +The wrong way to do it is code like: + +```kotlin +/** In N.kt (the network data type) */ +import C + +data class N() { + fun toC(): C { /* return a C created from a N */ } +} + +// --- + +/** in C.kt (the cache data type) */ +import N + +data class C() { + fun toN(): N { /* return a N created from a C */ } +} +``` + +This creates a loop in their dependency graph as they import each other's types. + +```mermaid +classDiagram + direction RL + class N{ + toC() C + } + + class C{ + toN() N + } + + C ..> N: Imports + N ..> C: Imports +``` + +This is a problem because: + +- They can't be placed in separate modules +- Modifying code in `N` can cause `C` to be recompiled, and vice-versa + +To fix this: + +1. Pick one type as being "higher" in the dependency tree than the other +2. Remove the `to...()` method from the lower type, and implement it as a companion `from()` method on the higher type + +In Pachli the dependency hierarchy is (higher types on the left): + +```mermaid +flowchart LR + ViewData --> ViewModel --> Cache --> Network --> Core +``` + +so the previous example involving a network type and a cache type would instead be written as: + +```kotlin +/** In N.kt (the network data type) */ +data class N() { + // No import, no toC() method +} + +// --- + +/** in C.kt (the cache data type) */ +import N + +data class C() { + fun toN(): N { /* return a N created from a C */ } + companion object { + fun from(n: N): C { + // code from the N.toC() in the previous example + } + } +} +``` + +Now the dependency between the two types is: + +```mermaid +classDiagram + direction RL + class N{ + } + + class C{ + toN() N + from(n: N)$ C + } + + C <.. N: Imports +``` + +The circular dependency is gone, the `N` type can easily be placed in a separate module to the `C` type, and changes to the `C` type will not require the `N` type to be recompiled. + +In these examples the `from` method could also have been written as a secondary constructor instead of a static factory method in the companion object. We prefer static factory methods over secondary constructors because: + +1. Functions have names that can more clearly indicate their intent +2. Functions can return objects of any subtype. If the example class `C` had multiple subtypes the correct subtype could be returned based on properties of `N` +3. Functions can return null or other values to signify an error. Perhaps the network type is expected to contain a particular property, but the server has a bug and returned data without that property. +4. Functions can have more specific visibility modifiers +5. Functions can be marked `inline` diff --git a/docs/contributing/code.md b/docs/contributing/code.md index 554ed6426..ad93f41f8 100644 --- a/docs/contributing/code.md +++ b/docs/contributing/code.md @@ -93,7 +93,7 @@ Pull requests (PRs) are the primary unit of collaboration for code. ### Work on branches in your own fork of the repository -Do not clone the `pachli-android` repository. Instead, create a fork, create a branch in your fork from the `main` branch, and commit your changes to that. +Do not clone the `pachli-android` repository. Instead, create a fork, create a branch in your fork from the `main` branch, and commit your changes to that branch. See the GitHub [Collaborating with pull requests](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/getting-started/about-collaborative-development-models) documentation. @@ -114,6 +114,32 @@ Typically you would configure the build variant in Android Studio with Build > S This is not mandatory, but may make developing easier for you. +### Code style + +#### `ktlintCheck` and `ktlintFormat` + +The project uses [ktlint](https://pinterest.github.io/ktlint/) to enforce common code and formatting standards. + +You can check your code before creating the PR with the `ktlintCheck` task. + +```shell +./gradlew ktlintCheck +``` + +Most code formatting issues can be automatically resolved with the `ktlintFormat` task. + +```shell +./gradlew ktlintFormat +``` + +The code in your PR will be checked for this every time it changes. If it is not lint-clean and automated fixes are possible they will be added as comments to the PR. + +#### Questions of taste + +Some code style issues are questions of taste, where developers might reasonably differ but the project has a specific stance. + +Please read the [Code style guide](/docs/code-style.md). + ### Individual commits A PR is typically made up multiple commits. @@ -148,7 +174,7 @@ This makes things needlessly difficult for your reviewers. The project uses the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard for commit messages. If you are not familiar with them [Conventional Commits: A better way](https://medium.com/neudesic-innovation/conventional-commits-a-better-way-78d6785c2e08) is also a good introduction. > [!NOTE] -> See [docs/decisions/0001-use-conventional-commits.md](https://github.com/pachli/pachli-android/docs/decisions/0001-use-conventional-commits.md) +> See [docs/decisions/0001-use-conventional-commits.md](/docs/decisions/0001-use-conventional-commits.md) The PR's title and description will become the first line and remaining body of the commit message when the PR is merged, so your PR title and description should also follow the conventional commits approach. @@ -175,7 +201,7 @@ The types are: - `test`, modify the test suite - `wip`, work-in-progress -More details on each is in [docs/decisions/conventional-commits.md](https://github.com/pachli/pachli-android/docs/decisions/conventional-commits.md). +More details on each is in [docs/decisions/0001-use-conventional-commits.md](/docs/decisions/0001-use-conventional-commits.md). `feat` for new features and `fix` for bug fixes are the most common. @@ -250,24 +276,6 @@ You should periodically merge changes from the `main` branch in to your PR branc If your PR can not be cleanly merged in to `main` it is difficult to review effectively, because merging the changes from `main` in to your PR will invalidate the review. You've changed the code, so the reviewer needs to look at it again. -#### `ktlintCheck` and `ktlintFormat` - -The project uses [ktlint](https://pinterest.github.io/ktlint/) to enforce common code and formatting standards. - -You can check your code before creating the PR with the `ktlintCheck` task. - -```shell -./gradlew ktlintCheck -``` - -Most code formatting issues can be automatically resolved with the `ktlintFormat` task. - -```shell -./gradlew ktlintFormat -``` - -The code in your PR will be checked for this every time it changes. If it is not lint-clean and automated fixes are possible they will be added as comments to the PR. - #### Tests The project has a number of automated tests, they will automatically be run on your PR when it is submitted.