Fix auto play when skipping last

[SergioEstevao]

| 📘 Part of: # |
|:---:|

Fixes #2019

Updates the code to make sure that auto-play logic and other end of episode logic is activated when skip last is triggered.

To test

• Start the app
• Ensure that you have Auto-Play active ( Profile -> Settings -> General -> Autoplay )
• Clean your Up Next Queue
• Subscribe to a podcast
• On the podcast page tap on the cog icon on the top right
• Setup a Skip Last time. Ex: 15 seconds
• Ensure that podcast has multiple episodes available
• Play an episode of the podcast
• Skip forward near to the skip last interval
• await for the skip last to kick in
• see if the auto-play works and a new episode of the same podcast is added to your queue and starts playing.

Checklist

• I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
• I have considered adding unit tests for my changes.
• I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

[dangermattic]

	1 Warning
⚠️	This PR is assigned to the milestone 7.71. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

[rviljoen]

@SergioEstevao Is the call to EpisodeManager.markAsPlayed() still needed? I think all the episode cleanup is already done via playerDidFinishPlayingEpisode().

Also, this will fix the bug for Skip Outro, but not the similar bug when episode is marked as played from Mini Player or Episode list. It would be good to fix all instances, if possible.

[leandroalonso]

@SergioEstevao as @rviljoen mentioned I think we have duplicate calls with this change.

Also, following his comment, we could try to fix that in all instances. I played a bit with it and came up with this diff.

It involves adding a new notification and handling it accordingly. This should fix all instances: the reported bug, marking as played (from miniplayer or anywhere else).

We would need to test a bit more to make sure we haven't introduced any regression though.

[rviljoen]

Hi @leandroalonso - I think your approach can work well. You will just need to do the same for EpisodeManager.archiveEpisode.

The other idea I had was to insert a call to autoPlayIfNeeded() in this method before line 551:

pocket-casts-ios/podcasts/PlaybackManager.swift

Lines 546 to 563 in 3747b79

	func removeIfPlayingOrQueued(episode: BaseEpisode?, fireNotification: Bool, saveCurrentEpisode: Bool = true, userInitiated: Bool = false) {
	if userInitiated, let episode {
	AnalyticsEpisodeHelper.shared.episodeRemovedFromUpNext(episode: episode)
	}
	if isNowPlayingEpisode(episodeUuid: episode?.uuid) {
	if queue.upNextCount() > 0 {
	playNextEpisode(autoPlay: playing())
	} else {
	endPlayback(saveCurrentEpisode: saveCurrentEpisode)
	}
	return
	}
	if let episode = episode {
	queue.remove(episode: episode, fireNotification: fireNotification)
	}
	}

I did some quick testing and I think it covers all the cases. In addition I think it is a low risk change, because the only thing that method call does is to add a single episode to the queue if the queue is empty and autoPlay is enabled. Rest of the app logic remains the same.

[leandroalonso]

I did some quick testing and I think it covers all the cases. In addition I think it is a low risk change, because the only thing that method call does is to add a single episode to the queue if the queue is empty and autoPlay is enabled. Rest of the app logic remains the same.

Oh, nice! Agree that's a best approach and less risky!

[SergioEstevao]

The other idea I had was to insert a call to autoPlayIfNeeded() in this method before line 551:

@rviljoen thanks for the suggestion

So you believe with that change we don't need to do any other change? or this is in conjunction with Leandro changes?

[rviljoen]

The other idea I had was to insert a call to autoPlayIfNeeded() in this method before line 551:

@rviljoen thanks for the suggestion

So you believe with that change we don't need to do any other change? or this is in conjunction with Leandro changes?

I think that one liner covers all the cases without the need for any other code changes. Please check my logic though 😄

[SergioEstevao]

@leandroalonso I update the code with @rviljoen suggestion. I tested it with the following scenarios:

• Skip Last activated and then let the time get to the skip period
• Mark as played on Episode List and Mini-Player
• Sleep time active to the end of episode and Skip last active

It looks it's working correctly on the above do you mind give it another look?

[rviljoen]

Looks good to me!

[leandroalonso]

@SergioEstevao one last thing: you can remove the autoplayIfNeeded() in line 978.

[rviljoen]

@SergioEstevao one last thing: you can remove the autoplayIfNeeded() in line 978.

I think that one has to stay in for the case where the episode plays to the end without skip outro and when episode is not mark as played/archived by the user.

[SergioEstevao]

I think that one has to stay in for the case where the episode plays to the end without skip outro and when episode is not mark as played/archived by the user.

I agree with you @rviljoen . I just tested the above and we need that line to be there for Auto-play to work

[leandroalonso]

Ops, my mistake. 🤦

Thank you both!