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 #5212: removing bug causing progress dialog to load forever #5350

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

TaiHaDev
Copy link
Contributor

Fixes: #5212

Description:
The bug originates from the updateDepictions function, which manages depiction updates and observes the result with both success and error handlers. While it does initiate the depiction update in the databases, the subsequent call to the showNotification function leads to an error. This issue arises because API levels 31 and above require a more specific flag for pending intents.

Changes:
Introduced an if statement to assess the current API version. If the API version exceeds 31, the flag is set to immutable.

Testing:
Verified on API levels 28 and 33 without any issues.

@@ -505,6 +505,7 @@ private void onMediaRefreshed(Media media) {
.subscribe(this::onDepictionsLoaded, Timber::e)
);
// compositeDisposable.add(disposable);

Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for this change? If not, would you mind removing the new line?

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 just added it by accident. I have removed it. Thank you!

@@ -224,9 +224,9 @@ class DepictsPresenter @Inject constructor(
Timber.e(
"Failed to update depictions"
)

Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Tested on several pictures, it works great!
https://commons.wikimedia.org/w/index.php?title=File:Ikuchijima_211.jpg&diff=prev&oldid=812977952

Source code looks good too.

@nicolas-raoul nicolas-raoul merged commit 988b83d into commons-app:v4.2.0-release Oct 19, 2023
@TaiHaDev TaiHaDev deleted the #5212_bug_fixing branch October 19, 2023 13:42
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.

2 participants