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

feat: Pre-release patches toggle #1785

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

kitadai31
Copy link
Contributor

@kitadai31 kitadai31 commented Mar 25, 2024

Add a toggle for pre-release patches to the alternative source settings.
Closes: #821

This PR also fixes an problem where pre-release Integrations are being used when using alternate sources.
At this time, Patches are referring GitHubAPI#getLatestPatchesRelease() that doesn't include pre-releases, but Integrations are referring GitHubAPI#getLatestRelease() that includes pre-releases.
This problem actually caused patched apps to crash recently in crimera/piko patches (an alternative source) when they released a pre-releases.
(Actually, this is the main motivation)

getLatestReleaseFile() in github_api.dart was removed because it's unused

@Ushie
Copy link
Member

Ushie commented Mar 25, 2024

Why limit it to alternative sources?

You can pass ?dev=true to achieve the same with the ReVanced API

@oSumAtrIX
Copy link
Member

Are changelogs/updates considered as well?

@Ushie I am not sure but are queries ignored by cache on CloudFlare? Because then, using the dev query has no effect.

@kitadai31
Copy link
Contributor Author

kitadai31 commented Mar 25, 2024

I already tried ?dev=true at ReVanced API, but it didn't effect.
https://api.revanced.app/v2/revanced-patches/releases/latest?dev=true
So, I thought it wasn't implemented yet.
That's why I limited to GitHub API only. I didn't know the cache issue.

If the cache issue will be fixed, I think I should wait for it.

Are changelogs/updates considered as well?

Yes. These lines change the data source of patches update dialog according to the settings.

Future<Map<String, dynamic>?> getLatestPatchesRelease() {
if (_managerAPI.isUsingAlternativeSources() &&
_managerAPI.isUsingPrereleasePatches()) {
return _githubAPI
.getLatestReleaseWithPreReleases(_managerAPI.getPatchesRepo());
} else {
return _githubAPI.getLatestRelease(_managerAPI.getPatchesRepo());
}
}

@brian6932
Copy link

brian6932 commented Apr 16, 2024

One thing I noticed after building this branch (rebased) was that checking for Revanced Manager updates gets stuck on Loading..., patches work fine though. Is that just a quirk of debug builds, or an issue with the PR?

@validcube validcube linked an issue Apr 18, 2024 that may be closed by this pull request
4 tasks
@nivranaitsirhc
Copy link

One thing I noticed after building this branch (rebased) was that checking for Revanced Manager updates gets stuck on Loading..., patches work fine though. Is that just a quirk of debug builds, or an issue with the PR?

Can you share the apk here please

@brian6932
Copy link

brian6932 commented Apr 20, 2024

@nivranaitsirhc sure [redacted]

@nivranaitsirhc
Copy link

nivranaitsirhc commented Apr 20, 2024

@nivranaitsirhc sure [redacted]

It works like a charm! Thanks!

Screenshot_20240420-191932_ReVanced Manager Debug.png

Screenshot_20240420-191959_ReVanced Manager Debug.png

Screenshot_20240420-193744_ReVanced Manager Debug.png

Screenshot_20240420-194113_YouTube.png

# Conflicts:
#	lib/services/github_api.dart
#	lib/services/manager_api.dart
#	lib/ui/views/home/home_viewmodel.dart
If there are multiple dev releases on the same day, they will be sorted by alphabetically. Therefore, releases[0] is not guaranteed to be the latest.
vX.X.X-dev.10 will be older than vX.X.X-dev.9
so we have to iterate several releases and find the latest
@kitadai31 kitadai31 marked this pull request as draft May 14, 2024 09:30
@kitadai31
Copy link
Contributor Author

kitadai31 commented May 14, 2024

Waiting for the parameter issue in ReVanced API to be fixed

I have some questions

  1. I added the ReVanced API v2 related logic in revanced_api.dart, but should I split the file into two files like revanced_api_v2.dart ?
  2. I added a word "(dev)" in settings description because ReVanced calls pre-release versions “dev” officially.
    Is this no problem? (40b1589)

If the stable version is newer than the most recent pre-release, then the stable version will be used, so make it clear.
Copy link
Member

@Ushie Ushie left a comment

Choose a reason for hiding this comment

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

@oSumAtrIX Does v3 suffer from the same issues regarding the ?dev parameter, supporting the API is a prerequisite for this PR

Comment on lines +40 to +67
Future<Map<String, dynamic>?> getLatestReleaseWithPreReleases(
String repoName,
) async {
try {
final response = await _dio.get('/repos/$repoName/releases?per_page=10');
final List<dynamic> releases = response.data;

/*
* Loop through all releases (including pre-releases) and return the latest
*/
Map<String, dynamic>? latestRelease;
DateTime latestReleaseDate = DateTime.fromMillisecondsSinceEpoch(0);
for (final release in releases) {
final DateTime releaseDate = DateTime.parse(release['published_at']);
if (releaseDate.isAfter(latestReleaseDate)) {
latestReleaseDate = releaseDate;
latestRelease = release;
}
}
return latestRelease;
} on Exception catch (e) {
if (kDebugMode) {
print(e);
}
return null;
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead of making a new function, add a parameter to the existing one instead

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Aug 7, 2024

A ?dev query parameter does intendedly not exist and is not planned to be added. The TLDR is that the dev branch is not a controlled release channel like a "beta" channel would be. If we were to want to add a beta channel, we would need do introduce significant changes as well as maintenance overhead as the beta channel would be a new production channel next to the main channel.

@Ushie
Copy link
Member

Ushie commented Aug 7, 2024

The dev branch has nothing to do with this, the flag can be renamed to ?prereleases instead of ?dev because it simply enables returning prereleases

@oSumAtrIX
Copy link
Member

The name does not change that pre releases are not a controlled production release channel. Any further discussion should happen in the API repo and not in this PR

@kitadai31
Copy link
Contributor Author

Yeah, I've heard that ReVanced API v2 was deprecated.
And v3 doesn't have toggles for pre-releases.

So, I think it's enough to revert this PR to the beginning. (Pre-release requires alternative sources to be enabled, #1785 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add feature/setting to pick pre-release patches instead feat: allow dev channel patches
5 participants