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 inconsistency between user interaction and database commit order when re-adding videos to the playlist #8248

Merged
merged 2 commits into from
Oct 3, 2023
Merged
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
25 changes: 24 additions & 1 deletion app/src/main/java/org/schabi/newpipe/fragments/MainFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.schabi.newpipe.databinding.FragmentMainBinding;
import org.schabi.newpipe.error.ErrorUtil;
import org.schabi.newpipe.extractor.exceptions.ExtractionException;
import org.schabi.newpipe.local.playlist.LocalPlaylistFragment;
import org.schabi.newpipe.settings.tabs.Tab;
import org.schabi.newpipe.settings.tabs.TabsManager;
import org.schabi.newpipe.util.NavigationHelper;
Expand Down Expand Up @@ -217,6 +218,12 @@ private void updateTitleForTab(final int tabPosition) {
setTitle(tabsList.get(tabPosition).getTabName(requireContext()));
}

public void commitPlaylistTabs() {
pagerAdapter.getLocalPlaylistFragments()
.stream()
.forEach(LocalPlaylistFragment::commitChanges);
}

private void updateTabLayoutPosition() {
final ScrollableTabLayout tabLayout = binding.mainTabLayout;
final ViewPager viewPager = binding.pager;
Expand Down Expand Up @@ -268,10 +275,18 @@ public void onTabReselected(final TabLayout.Tab tab) {
updateTitleForTab(tab.getPosition());
}

private static final class SelectedTabsPagerAdapter
public static final class SelectedTabsPagerAdapter
extends FragmentStatePagerAdapterMenuWorkaround {
private final Context context;
private final List<Tab> internalTabsList;
/**
* Keep reference to LocalPlaylistFragments, because their data can be modified by the user
* during runtime and changes are not committed immediately. However, in some cases,
* the changes need to be committed immediately by calling
* {@link LocalPlaylistFragment#commitChanges()}.
* The fragments are removed when {@link LocalPlaylistFragment#onDestroy()} is called.
*/
private final List<LocalPlaylistFragment> localPlaylistFragments = new ArrayList<>();

private SelectedTabsPagerAdapter(final Context context,
final FragmentManager fragmentManager,
Expand All @@ -298,9 +313,17 @@ public Fragment getItem(final int position) {
((BaseFragment) fragment).useAsFrontPage(true);
}

if (fragment instanceof LocalPlaylistFragment) {
localPlaylistFragments.add((LocalPlaylistFragment) fragment);
}

return fragment;
}

public List<LocalPlaylistFragment> getLocalPlaylistFragments() {
return localPlaylistFragments;
}

@Override
public int getItemPosition(@NonNull final Object object) {
// Causes adapter to reload all Fragments when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import androidx.appcompat.widget.Toolbar;
import androidx.coordinatorlayout.widget.CoordinatorLayout;
import androidx.core.content.ContextCompat;
import androidx.fragment.app.Fragment;
import androidx.preference.PreferenceManager;

import com.google.android.exoplayer2.PlaybackException;
Expand Down Expand Up @@ -84,11 +85,13 @@
import org.schabi.newpipe.fragments.BackPressable;
import org.schabi.newpipe.fragments.BaseStateFragment;
import org.schabi.newpipe.fragments.EmptyFragment;
import org.schabi.newpipe.fragments.MainFragment;
import org.schabi.newpipe.fragments.list.comments.CommentsFragment;
import org.schabi.newpipe.fragments.list.videos.RelatedItemsFragment;
import org.schabi.newpipe.ktx.AnimationType;
import org.schabi.newpipe.local.dialog.PlaylistDialog;
import org.schabi.newpipe.local.history.HistoryRecordManager;
import org.schabi.newpipe.local.playlist.LocalPlaylistFragment;
import org.schabi.newpipe.player.Player;
import org.schabi.newpipe.player.PlayerService;
import org.schabi.newpipe.player.PlayerType;
Expand Down Expand Up @@ -472,10 +475,23 @@ private void setOnClickListeners() {

binding.detailControlsBackground.setOnClickListener(v -> openBackgroundPlayer(false));
binding.detailControlsPopup.setOnClickListener(v -> openPopupPlayer(false));
binding.detailControlsPlaylistAppend.setOnClickListener(makeOnClickListener(info ->
binding.detailControlsPlaylistAppend.setOnClickListener(makeOnClickListener(info -> {
if (getFM() != null && currentInfo != null) {
final Fragment fragment = getParentFragmentManager().
findFragmentById(R.id.fragment_holder);

// commit previous pending changes to database
if (fragment instanceof LocalPlaylistFragment) {
((LocalPlaylistFragment) fragment).commitChanges();
} else if (fragment instanceof MainFragment) {
((MainFragment) fragment).commitPlaylistTabs();
}

disposables.add(PlaylistDialog.createCorrespondingDialog(requireContext(),
List.of(new StreamEntity(info)),
dialog -> dialog.show(getParentFragmentManager(), TAG)))));
dialog -> dialog.show(getParentFragmentManager(), TAG)));
}
}));
binding.detailControlsDownload.setOnClickListener(v -> {
if (PermissionHelper.checkStoragePermissions(activity,
PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.schabi.newpipe.error.ErrorInfo;
import org.schabi.newpipe.error.UserAction;
import org.schabi.newpipe.extractor.stream.StreamInfoItem;
import org.schabi.newpipe.fragments.MainFragment;
import org.schabi.newpipe.fragments.list.playlist.PlaylistControlViewHolder;
import org.schabi.newpipe.info_list.dialog.InfoItemDialog;
import org.schabi.newpipe.info_list.dialog.StreamDialogDefaultEntry;
Expand Down Expand Up @@ -71,7 +72,7 @@

public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistStreamEntry>, Void>
implements PlaylistControlViewHolder {
// Save the list 10 seconds after the last change occurred
/** Save the list 10 seconds after the last change occurred. */
private static final long SAVE_DEBOUNCE_MILLIS = 10000;
private static final int MINIMUM_INITIAL_DRAG_VELOCITY = 12;
@State
Expand All @@ -92,13 +93,20 @@ public class LocalPlaylistFragment extends BaseLocalListFragment<List<PlaylistSt
private PublishSubject<Long> debouncedSaveSignal;
private CompositeDisposable disposables;

/* Has the playlist been fully loaded from db */
/** Whether the playlist has been fully loaded from db. */
private AtomicBoolean isLoadingComplete;
/* Has the playlist been modified (e.g. items reordered or deleted) */
/** Whether the playlist has been modified (e.g. items reordered or deleted) */
private AtomicBoolean isModified;
/* Flag to prevent simultaneous rewrites of the playlist */
/** Flag to prevent simultaneous rewrites of the playlist. */
private boolean isRewritingPlaylist = false;

/**
* The pager adapter that the fragment is created from when it is used as frontpage, i.e.
* {@link #useAsFrontPage} is {@link true}.
*/
@Nullable
private MainFragment.SelectedTabsPagerAdapter tabsPagerAdapter = null;

public static LocalPlaylistFragment getInstance(final long playlistId, final String name) {
final LocalPlaylistFragment instance = new LocalPlaylistFragment();
instance.setInitialData(playlistId, name);
Expand Down Expand Up @@ -158,6 +166,17 @@ protected ViewBinding getListHeader() {
return headerBinding;
}

/**
* <p>Commit changes immediately if the playlist has been modified.</p>
* Delete operations and other modifications will be committed to ensure that the database
* is up to date, e.g. when the user adds the just deleted stream from another fragment.
*/
public void commitChanges() {
if (isModified != null && isModified.get()) {
saveImmediate();
}
}

@Override
protected void initListeners() {
super.initListeners();
Expand Down Expand Up @@ -291,6 +310,9 @@ public void onDestroy() {
if (disposables != null) {
disposables.dispose();
}
if (tabsPagerAdapter != null) {
tabsPagerAdapter.getLocalPlaylistFragments().remove(this);
}

debouncedSaveSignal = null;
playlistManager = null;
Expand Down Expand Up @@ -877,5 +899,10 @@ private void createShareConfirmationDialog() {
)
.show();
}

public void setTabsPagerAdapter(
@Nullable final MainFragment.SelectedTabsPagerAdapter tabsPagerAdapter) {
this.tabsPagerAdapter = tabsPagerAdapter;
}
}

Loading