Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3771 mark post as reply #3778

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ private void bindViewHolder(@NonNull RecyclerView.ViewHolder viewHolder, int pos
if (payloads == null) {
holder.showStatusContent(true);
}
holder.setupWithStatus(status, statusListener, statusDisplayOptions, payloadForHolder);
holder.setupWithStatus(status, statusListener, statusDisplayOptions, payloadForHolder, true);
}
if (concreteNotification.getType() == Notification.Type.POLL) {
holder.setPollInfo(accountId.equals(concreteNotification.getAccount().getId()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,15 +399,6 @@ private CharSequence getCreatedAtDescription(Date createdAt,
}
}

protected void setIsReply(boolean isReply) {
if (isReply) {
replyButton.setImageResource(R.drawable.ic_reply_all_24dp);
} else {
replyButton.setImageResource(R.drawable.ic_reply_24dp);
}

}

protected void setReplyCount(int repliesCount, boolean fullStats) {
// This label only exists in the non-detailed view (to match the web ui)
if (replyCountLabel == null) return;
Expand Down Expand Up @@ -769,20 +760,20 @@ private void showConfirmFavourite(StatusActionListener listener,
}

public void setupWithStatus(@NonNull StatusViewData.Concrete status, final @NonNull StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions) {
this.setupWithStatus(status, listener, statusDisplayOptions, null);
@NonNull StatusDisplayOptions statusDisplayOptions, boolean showStatusInfo) {
this.setupWithStatus(status, listener, statusDisplayOptions, null, showStatusInfo);
}

public void setupWithStatus(@NonNull StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads) {
@Nullable Object payloads,
boolean showStatusInfo) {
if (payloads == null) {
Status actionable = status.getActionable();
setDisplayName(actionable.getAccount().getName(), actionable.getAccount().getEmojis(), statusDisplayOptions);
setUsername(actionable.getAccount().getUsername());
setMetaData(status, statusDisplayOptions, listener);
setIsReply(actionable.getInReplyToId() != null);
setReplyCount(actionable.getRepliesCount(), statusDisplayOptions.showStatsInline());
setAvatar(actionable.getAccount().getAvatar(), status.getRebloggedAvatar(),
actionable.getAccount().getBot(), statusDisplayOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,14 @@ private void setReblogAndFavCount(int reblogCount, int favCount, StatusActionLis
public void setupWithStatus(@NonNull final StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads) {
@Nullable Object payloads,
boolean showStatusInfo) {
// We never collapse statuses in the detail view
StatusViewData.Concrete uncollapsedStatus = (status.isCollapsible() && status.isCollapsed()) ?
status.copyWithCollapsed(false) :
status;

super.setupWithStatus(uncollapsedStatus, listener, statusDisplayOptions, payloads);
super.setupWithStatus(uncollapsedStatus, listener, statusDisplayOptions, payloads, showStatusInfo);
setupCard(uncollapsedStatus, status.isExpanded(), CardViewMode.FULL_WIDTH, statusDisplayOptions, listener); // Always show card for detailed status
if (payloads == null) {
Status actionable = uncollapsedStatus.getActionable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.keylesspalace.tusky.entity.Emoji;
import com.keylesspalace.tusky.entity.Filter;
import com.keylesspalace.tusky.entity.Status;
import com.keylesspalace.tusky.entity.TimelineAccount;
import com.keylesspalace.tusky.interfaces.StatusActionListener;
import com.keylesspalace.tusky.util.CustomEmojiHelper;
import com.keylesspalace.tusky.util.NumberUtils;
Expand All @@ -38,6 +39,7 @@
import com.keylesspalace.tusky.util.StringUtils;
import com.keylesspalace.tusky.viewdata.StatusViewData;

import java.util.Collections;
import java.util.List;

import at.connyduck.sparkbutton.helpers.Utils;
Expand All @@ -63,21 +65,37 @@ public StatusViewHolder(@NonNull View itemView) {
public void setupWithStatus(@NonNull StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads) {
@Nullable Object payloads,
boolean showStatusInfo) {
if (payloads == null) {

boolean sensitive = !TextUtils.isEmpty(status.getActionable().getSpoilerText());
boolean expanded = status.isExpanded();

setupCollapsedState(sensitive, expanded, status, listener);

Status reblogging = status.getRebloggingStatus();
if (reblogging == null || status.getFilterAction() == Filter.Action.WARN) {
boolean isReply = status.getStatus().getInReplyToId() != null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to use status.getActionable here, and on line 90?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was searching for some time for the right location with the right information.
"actionable" was not it if I remember correctly.

It is working and plausible at the moment.

(Would need to look deep(er) into the code if you want me to be 100% certain about this.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, the inreplyto timeline account is only available on status, but without the timeline type mapper change, the correct thing to check for the inreplyto id is the actionable status. (And if status.getActionable().getInReplyToId() is nonnull, then status.getInReplyToAccount() will be as well.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the meaning of "actionable" in current code?

I see that it only has a value if "reblog" is true (Status.kt:59), so the thinking is probably "the item the action was taken for"?

(The "reply stuff" I have aligned more with the extsting "inReplyToAccountId".)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my understanding is that the "outer" status is the boost itself, and the actionable status is the status that was boosted

boolean isReplyOnly = isReply && reblogging == null;

boolean hasStatusContext = reblogging != null || isReply;

if (!hasStatusContext || !showStatusInfo || status.getFilterAction() == Filter.Action.WARN) {
hideStatusInfo();
} else {
String rebloggedByDisplayName = reblogging.getAccount().getName();
setRebloggedByDisplayName(rebloggedByDisplayName,
reblogging.getAccount().getEmojis(), statusDisplayOptions);
String accountName = "";
List<Emoji> emojis = Collections.emptyList();
if (reblogging != null) {
accountName = reblogging.getAccount().getName();
emojis = reblogging.getAccount().getEmojis();
} else if (isReply) {
TimelineAccount repliedTo = status.getInReplyToAccount();
if (repliedTo != null) {
accountName = repliedTo.getName();
emojis = repliedTo.getEmojis();
}
}

setStatusInfoText(isReplyOnly, accountName, emojis, statusDisplayOptions);
statusInfo.setOnClickListener(v -> listener.onOpenReblog(getBindingAdapterPosition()));
}

Expand All @@ -88,19 +106,27 @@ public void setupWithStatus(@NonNull StatusViewData.Concrete status,
setFavouritedCount(status.getActionable().getFavouritesCount());
setReblogsCount(status.getActionable().getReblogsCount());

super.setupWithStatus(status, listener, statusDisplayOptions, payloads);
super.setupWithStatus(status, listener, statusDisplayOptions, payloads, showStatusInfo);
}

private void setRebloggedByDisplayName(final CharSequence name,
final List<Emoji> accountEmoji,
final StatusDisplayOptions statusDisplayOptions) {
private void setStatusInfoText(final boolean isReply,
final CharSequence name,
final List<Emoji> accountEmoji,
final StatusDisplayOptions statusDisplayOptions) {

Context context = statusInfo.getContext();
CharSequence wrappedName = StringUtils.unicodeWrap(name);
CharSequence boostedText = context.getString(R.string.post_boosted_format, wrappedName);
CharSequence emojifiedText = CustomEmojiHelper.emojify(
boostedText, accountEmoji, statusInfo, statusDisplayOptions.animateEmojis()
);
statusInfo.setText(emojifiedText);
if (name.length() > 0) {
CharSequence wrappedName = StringUtils.unicodeWrap(name);
CharSequence statusContextText = context.getString(isReply ? R.string.post_replied_format : R.string.post_boosted_format, wrappedName);
CharSequence emojifiedText = CustomEmojiHelper.emojify(
statusContextText, accountEmoji, statusInfo, statusDisplayOptions.animateEmojis()
);
statusInfo.setText(emojifiedText);
} else {
statusInfo.setText(context.getString(R.string.post_replied));
}
statusInfo.setCompoundDrawablesWithIntrinsicBounds(isReply ? R.drawable.ic_reply_all_18dp : R.drawable.ic_reblog_18dp, 0, 0, 0);

statusInfo.setVisibility(View.VISIBLE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ data class ConversationStatusEntity(
language = language,
filtered = emptyList()
),
// TODO? implementation gap: not needed here atm, but inconsistent
inReplyToAccount = null,
isExpanded = expanded,
isShowingContent = showingHiddenContent,
isCollapsed = collapsed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ void setupWithConversation(
setDisplayName(account.getDisplayName(), account.getEmojis(), statusDisplayOptions);
setUsername(account.getUsername());
setMetaData(statusViewData, statusDisplayOptions, listener);
setIsReply(status.getInReplyToId() != null);
setFavourited(status.getFavourited());
setBookmarked(status.getBookmarked());
List<Attachment> attachments = status.getAttachments();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class SearchStatusesAdapter(

override fun onBindViewHolder(holder: StatusViewHolder, position: Int) {
getItem(position)?.let { item ->
holder.setupWithStatus(item, statusListener, statusDisplayOptions)
holder.setupWithStatus(item, statusListener, statusDisplayOptions, true)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ class TimelinePagingAdapter(
status,
statusListener,
statusDisplayOptions,
if (payloads != null && payloads.isNotEmpty()) payloads[0] else null
if (payloads != null && payloads.isNotEmpty()) payloads[0] else null,
true
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ fun TimelineStatusWithAccount.toViewData(moshi: Moshi, isDetailed: Boolean = fal
// no url for reblogs
url = null,
account = this.reblogAccount!!.toAccount(moshi),
inReplyToId = null,
inReplyToAccountId = null,
inReplyToId = status.inReplyToId,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be consistent (a boost can also be a reply).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the boost itself is not the reply, right? The actionable status is the reply

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is boosted ("status") can be a reply (too).

This info is "unused".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now, this is here as a hack to populate the inreplyto timeline account of the concrete statusviewdata when it's a boost of a reply.
…but as you say, we're not displaying that information, so it could be reverted to avoid confusing the issue (although the status/actionable thing is already confusing)

inReplyToAccountId = status.inReplyToAccountId,
reblog = reblog,
content = "",
// lie but whatever?
Expand Down Expand Up @@ -269,6 +269,7 @@ fun TimelineStatusWithAccount.toViewData(moshi: Moshi, isDetailed: Boolean = fal
}
return StatusViewData.Concrete(
status = status,
inReplyToAccount = this.inReplyToAccount?.toAccount(moshi),
isExpanded = this.status.expanded,
isShowingContent = this.status.contentShowing,
isCollapsed = this.status.contentCollapsed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,27 @@ import androidx.paging.ExperimentalPagingApi
import androidx.paging.LoadType
import androidx.paging.PagingState
import androidx.paging.RemoteMediator
import com.keylesspalace.tusky.components.timeline.toAccount
import com.keylesspalace.tusky.components.timeline.util.ifExpected
import com.keylesspalace.tusky.db.AccountManager
import com.keylesspalace.tusky.db.AppDatabase
import com.keylesspalace.tusky.entity.TimelineAccount
import com.keylesspalace.tusky.util.HttpHeaderLink
import com.keylesspalace.tusky.util.toViewData
import com.keylesspalace.tusky.viewdata.StatusViewData
import com.squareup.moshi.Moshi
import retrofit2.HttpException

@OptIn(ExperimentalPagingApi::class)
class NetworkTimelineRemoteMediator(
private val accountManager: AccountManager,
private val viewModel: NetworkTimelineViewModel
private val viewModel: NetworkTimelineViewModel,
db: AppDatabase,
private val moshi: Moshi,
) : RemoteMediator<String, StatusViewData>() {

private val accountDao = db.timelineAccountDao()

private val statusIds = mutableSetOf<String>()

init {
Expand Down Expand Up @@ -80,7 +88,13 @@ class NetworkTimelineRemoteMediator(
val expanded = oldStatus?.isExpanded ?: activeAccount.alwaysOpenSpoiler
val contentCollapsed = oldStatus?.isCollapsed ?: true

var inReplyToAccount: TimelineAccount? = null
if (status.inReplyToAccountId != null) {
inReplyToAccount = accountDao.get(status.inReplyToAccountId)?.toAccount(moshi)
}

status.toViewData(
inReplyToAccount = inReplyToAccount,
isShowingContent = contentShowing,
isExpanded = expanded,
isCollapsed = contentCollapsed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import at.connyduck.calladapter.networkresult.onFailure
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.components.timeline.util.ifExpected
import com.keylesspalace.tusky.db.AccountManager
import com.keylesspalace.tusky.db.AppDatabase
import com.keylesspalace.tusky.entity.Filter
import com.keylesspalace.tusky.entity.Poll
import com.keylesspalace.tusky.entity.Status
Expand All @@ -41,6 +42,7 @@ import com.keylesspalace.tusky.util.isLessThanOrEqual
import com.keylesspalace.tusky.util.toViewData
import com.keylesspalace.tusky.viewdata.StatusViewData
import com.keylesspalace.tusky.viewdata.TranslationViewData
import com.squareup.moshi.Moshi
import java.io.IOException
import javax.inject.Inject
import kotlinx.coroutines.Dispatchers
Expand All @@ -60,7 +62,9 @@ class NetworkTimelineViewModel @Inject constructor(
eventHub: EventHub,
accountManager: AccountManager,
sharedPreferences: SharedPreferences,
filterModel: FilterModel
filterModel: FilterModel,
private val db: AppDatabase,
private val moshi: Moshi,
) : TimelineViewModel(
timelineCases,
api,
Expand All @@ -86,7 +90,7 @@ class NetworkTimelineViewModel @Inject constructor(
currentSource = source
}
},
remoteMediator = NetworkTimelineRemoteMediator(accountManager, this)
remoteMediator = NetworkTimelineRemoteMediator(accountManager, this, db, moshi)
).flow
.map { pagingData ->
pagingData.filter(Dispatchers.Default.asExecutor()) { statusViewData ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class ThreadAdapter(

override fun onBindViewHolder(viewHolder: StatusBaseViewHolder, position: Int) {
val status = getItem(position)
viewHolder.setupWithStatus(status, statusActionListener, statusDisplayOptions)
viewHolder.setupWithStatus(status, statusActionListener, statusDisplayOptions, false)
}

override fun getItemViewType(position: Int): Int {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public abstract class AppDatabase extends RoomDatabase {
@NonNull public abstract ConversationsDao conversationDao();
@NonNull public abstract TimelineDao timelineDao();
@NonNull public abstract DraftDao draftDao();
@NonNull public abstract TimelineAccountDao timelineAccountDao();

public static final Migration MIGRATION_2_3 = new Migration(2, 3) {
@Override
Expand Down
25 changes: 25 additions & 0 deletions app/src/main/java/com/keylesspalace/tusky/db/TimelineAccountDao.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/* Copyright 2023 Tusky Contributors
*
* This file is a part of Tusky.
*
* 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.
*
* Tusky 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 <http://www.gnu.org/licenses>. */

package com.keylesspalace.tusky.db

import androidx.room.Dao
import androidx.room.Query

@Dao
interface TimelineAccountDao {
@Query("SELECT * FROM TimelineAccountEntity WHERE serverId = :id")
suspend fun get(id: String): TimelineAccountEntity?
}
Loading