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

Fix skipping in thread pagination #2658

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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 @@ -78,4 +78,14 @@ default Guild getGuild()
{
return getChannel().getGuild();
}

/**
* {@inheritDoc}
*
* <b>Note:</b> When paginating over public and not-joined private threads,
* this skips based on {@link ThreadChannel#getTimeArchiveInfoLastModified() the archive timestamp}.
*/
@Nonnull
@Override
ThreadChannelPaginationAction skipTo(long id);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
import net.dv8tion.jda.api.requests.Response;
import net.dv8tion.jda.api.requests.Route;
import net.dv8tion.jda.api.requests.restaction.pagination.ThreadChannelPaginationAction;
import net.dv8tion.jda.api.utils.TimeUtil;
import net.dv8tion.jda.api.utils.data.DataArray;
import net.dv8tion.jda.api.utils.data.DataObject;
import net.dv8tion.jda.internal.entities.EntityBuilder;
import net.dv8tion.jda.internal.entities.GuildImpl;
import net.dv8tion.jda.internal.utils.Helpers;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -59,19 +61,16 @@ protected String getPaginationLastEvaluatedKey(long lastId, ThreadChannel last)
if (useID)
return Long.toUnsignedString(lastId);

if (order == PaginationOrder.FORWARD && lastId == 0)
{
// first second of 2015 aka discords epoch, hard coding something older makes no sense to me
return "2015-01-01T00:00:00.000";
}

// this should be redundant, due to calling this with PaginationAction#getLast() as last param,
// but let's have this here.
if (last == null)
if (last != null)
// If threads were retrieved
// OffsetDateTime#toString() is defined to be ISO8601, needs no helper method.
return last.getTimeArchiveInfoLastModified().toString();
else if (lastId != 0)
// If the user skipped ahead
return TimeUtil.getTimeCreated(lastId).toString();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this should even be allowed, it's very easy for the user to try to skip using an ID, when in reality it uses the ID as a iso8601 timestamp, requiring to pass the thread archival timestamp, instead of the thread ID.

My use case was to retrieve a channel by ID by making a small window with the pagination, would be much better if JDA had a way to retrieve a channel by ID

else
// If we just started paginating
return OffsetDateTime.now(ZoneOffset.UTC).toString();

// OffsetDateTime#toString() is defined to be ISO8601, needs no helper method.
return last.getTimeArchiveInfoLastModified().toString();
}

@Override
Expand Down Expand Up @@ -102,7 +101,7 @@ protected void handleSuccess(Response response, Request<List<ThreadChannel>> req

try
{
ThreadChannel thread = builder.createThreadChannel(threadObj, getGuild().getIdLong());
ThreadChannel thread = builder.createThreadChannel((GuildImpl) getGuild(), threadObj, getGuild().getIdLong(), false);
list.add(thread);

if (this.useCache)
Expand Down
Loading