jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/327005 )
Change subject: Hygiene: Clean up misuse of PageTitles in GalleryActivity
......................................................................
Hygiene: Clean up misuse of PageTitles in GalleryActivity
The logger expects a PageTitle object coming from the gallery activity,
but we may not be in the context of wiki page since the gallery can now
also be launched from the feed featured image card.
An earlier commit introduced the idea of keeping around a placeholder
PageTitle for logging purposes in the event we're coming from the feed
featured image card. But storing this in the pageTitle member variable
is a bad idea since this makes creates a risk it'll end up getting
inappropriately passed to the content model for the page data client or
inserted into the cache.
This patch eliminates such use of the PageTitle class and updates
downstream usages of the PageTitle object as need.
To follow up: figure out a good way to pass the featured image date
through to the gallery for use in a share subject. A
FeaturedImageGalleryItem subclass would likely be appropriate here.
Bug: T152980
Change-Id: I74ef3557700018176310097180515664652b13ca
---
M app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
M app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
M app/src/main/java/org/wikipedia/gallery/GalleryItemFragment.java
M app/src/main/java/org/wikipedia/main/MainFragment.java
M app/src/main/java/org/wikipedia/util/ShareUtil.java
5 files changed, 42 insertions(+), 36 deletions(-)
Approvals:
Niedzielski: Looks good to me, approved
jenkins-bot: Verified
diff --git a/app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
b/app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
index 4403667..067dbd5 100644
--- a/app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
+++ b/app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
@@ -38,7 +38,9 @@
private void logGalleryAction(String action, PageTitle currentPageTitle,
String currentMediaTitle) {
log(
"action", action,
- "pageTitle", currentPageTitle.getDisplayText(),
+ "pageTitle", currentPageTitle != null
+ ? currentPageTitle.getDisplayText()
+ : "FeedFeaturedImage",
"imageTitle", currentMediaTitle
);
}
diff --git a/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
b/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
index bd415a0..794d746 100644
--- a/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
+++ b/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
@@ -138,23 +138,26 @@
};
@NonNull
- public static Intent newIntent(@NonNull Context context, @NonNull
PageTitle pageTitle,
- @NonNull String filename, @NonNull WikiSite
wiki, int source,
+ public static Intent newIntent(@NonNull Context context, @NonNull String
filename,
+ @NonNull WikiSite wiki, int source,
@NonNull FeaturedImage image) {
- return newIntent(context, pageTitle, filename, wiki, source)
- .putExtra(EXTRA_FEATURED_IMAGE, GsonMarshaller.marshal(image))
- .putExtra(EXTRA_IS_FEATURED_IMAGE, true);
+ return newIntent(context, null, filename, wiki,
+ source).putExtra(EXTRA_FEATURED_IMAGE,
GsonMarshaller.marshal(image));
}
@NonNull
- public static Intent newIntent(@NonNull Context context, @NonNull
PageTitle pageTitle,
+ public static Intent newIntent(@NonNull Context context, @Nullable
PageTitle pageTitle,
@NonNull String filename, @NonNull WikiSite
wiki, int source) {
- return new Intent()
+ Intent intent = new Intent()
.setClass(context, GalleryActivity.class)
.putExtra(EXTRA_FILENAME, filename)
.putExtra(EXTRA_WIKI, wiki)
- .putExtra(EXTRA_PAGETITLE, pageTitle)
.putExtra(EXTRA_SOURCE, source);
+ if (pageTitle != null) {
+ intent.putExtra(EXTRA_PAGETITLE, pageTitle);
+ }
+
+ return intent;
}
@Override
@@ -185,7 +188,9 @@
creditText = (TextView) findViewById(R.id.gallery_credit_text);
creditText.setShadowLayer(2, 1, 1, color(R.color.lead_text_shadow));
- pageTitle = getIntent().getParcelableExtra(EXTRA_PAGETITLE);
+ if (getIntent().hasExtra(EXTRA_PAGETITLE)) {
+ pageTitle = getIntent().getParcelableExtra(EXTRA_PAGETITLE);
+ }
initialFilename = getIntent().getStringExtra(EXTRA_FILENAME);
wiki = getIntent().getParcelableExtra(EXTRA_WIKI);
@@ -227,15 +232,10 @@
updateProgressBar(false, true, 0);
- if (pageTitle == null) {
- throw new IllegalStateException("pageTitle should not be null");
- } else if (getIntent().hasExtra(EXTRA_IS_FEATURED_IMAGE)
- && getIntent().getBooleanExtra(EXTRA_IS_FEATURED_IMAGE,
false)) {
+ if (getIntent().hasExtra(EXTRA_FEATURED_IMAGE)) {
FeaturedImage featuredImage =
GsonUnmarshaller.unmarshal(FeaturedImage.class,
getIntent().getStringExtra(EXTRA_FEATURED_IMAGE));
- if (featuredImage != null) {
- loadGalleryItemFor(featuredImage);
- }
+ loadGalleryItemFor(featuredImage);
} else {
// find our Page in the page cache...
app.getPageCache().get(pageTitle, 0, new
PageCache.CacheGetListener() {
@@ -269,7 +269,7 @@
super.onDestroy();
}
- private void loadGalleryItemFor(FeaturedImage image) {
+ private void loadGalleryItemFor(@NonNull FeaturedImage image) {
List<GalleryItem> list = new ArrayList<>();
list.add(new GalleryItem(image));
applyGalleryCollection(new GalleryCollection(list));
@@ -418,12 +418,12 @@
* Retrieve the complete list of media items for the current page.
* When retrieved, the list will be passed to the ViewPager, and will
become a
* scrollable gallery of media.
- *
- * WARNING: This may be useful if/when we start loading multiple images
into the gallery from
- * the feed, but it won't work with the current dummy PageTitle we're
using when coming from
- * the feed.
*/
private void fetchGalleryCollection() {
+ if (pageTitle == null) {
+ return;
+ }
+
updateProgressBar(true, true, 0);
new
GalleryCollectionFetchTask(app.getAPIForSite(pageTitle.getWikiSite()),
pageTitle.getWikiSite(), pageTitle) {
@@ -450,7 +450,7 @@
* Apply a complete collection of media to our scrollable gallery.
* @param collection GalleryCollection to apply to the ViewPager.
*/
- protected void applyGalleryCollection(GalleryCollection collection) {
+ protected void applyGalleryCollection(@NonNull GalleryCollection
collection) {
// remove the page transformer while we operate on the pager...
galleryPager.setPageTransformer(false, null);
// first, verify that the collection contains the item that the user
@@ -582,7 +582,7 @@
fragmentArray = new SparseArray<>();
}
- public void setCollection(GalleryCollection collection) {
+ public void setCollection(@NonNull GalleryCollection collection) {
galleryCollection = collection;
notifyDataSetChanged();
}
diff --git a/app/src/main/java/org/wikipedia/gallery/GalleryItemFragment.java
b/app/src/main/java/org/wikipedia/gallery/GalleryItemFragment.java
index dd44278..4fd4fbc 100644
--- a/app/src/main/java/org/wikipedia/gallery/GalleryItemFragment.java
+++ b/app/src/main/java/org/wikipedia/gallery/GalleryItemFragment.java
@@ -418,7 +418,9 @@
ShareUtil.shareImage(parentActivity,
bitmap,
new File(galleryItem.getUrl()).getName(),
- pageTitle.getDisplayText(),
+ pageTitle != null
+ ? pageTitle.getDisplayText()
+ :
ShareUtil.getFeaturedImageShareSubject(getContext(), null),
imageTitle.getCanonicalUri());
} else {
ShareUtil.shareText(parentActivity, imageTitle);
diff --git a/app/src/main/java/org/wikipedia/main/MainFragment.java
b/app/src/main/java/org/wikipedia/main/MainFragment.java
index 3f9111c..7a0ca03 100644
--- a/app/src/main/java/org/wikipedia/main/MainFragment.java
+++ b/app/src/main/java/org/wikipedia/main/MainFragment.java
@@ -31,7 +31,6 @@
import org.wikipedia.activity.FragmentUtil;
import org.wikipedia.analytics.GalleryFunnel;
import org.wikipedia.analytics.IntentFunnel;
-import org.wikipedia.dataclient.WikiSite;
import org.wikipedia.feed.FeedFragment;
import org.wikipedia.feed.image.FeaturedImage;
import org.wikipedia.feed.image.FeaturedImageCard;
@@ -56,7 +55,6 @@
import org.wikipedia.search.SearchInvokeSource;
import org.wikipedia.settings.Prefs;
import org.wikipedia.util.ClipboardUtil;
-import org.wikipedia.util.DateUtil;
import org.wikipedia.util.FeedbackUtil;
import org.wikipedia.util.PermissionUtil;
import org.wikipedia.util.ShareUtil;
@@ -252,7 +250,7 @@
ShareUtil.shareImage(getContext(),
bitmap,
new File(thumbUrl).getName(),
- featuredImageShareSubject(card),
+
ShareUtil.getFeaturedImageShareSubject(getContext(), card),
fullSizeUrl);
} else {
FeedbackUtil.showMessage(MainFragment.this,
getString(R.string.gallery_share_error, card.baseImage().title()));
@@ -271,9 +269,8 @@
}
@Override public void onFeaturedImageSelected(FeaturedImageCard card) {
- final WikiSite wiki = card.wikiSite();
- startActivityForResult(GalleryActivity.newIntent(getActivity(), new
PageTitle(featuredImageShareSubject(card), wiki),
- card.filename(), wiki,
GalleryFunnel.SOURCE_FEED_FEATURED_IMAGE, card.baseImage()),
+ startActivityForResult(GalleryActivity.newIntent(getActivity(),
card.filename(),
+ card.wikiSite(), GalleryFunnel.SOURCE_FEED_FEATURED_IMAGE,
card.baseImage()),
Constants.ACTIVITY_REQUEST_GALLERY);
}
@@ -470,11 +467,6 @@
if (behavior != null) {
behavior.onNestedFling(coordinatorLayout, paddingAppBar, null, 0,
params.height, true);
}
- }
-
- private String featuredImageShareSubject(FeaturedImageCard card) {
- return getString(R.string.feed_featured_image_share_subject) + " | "
- + DateUtil.getFeedCardDateString(card.date().baseCalendar());
}
@Nullable private Callback callback() {
diff --git a/app/src/main/java/org/wikipedia/util/ShareUtil.java
b/app/src/main/java/org/wikipedia/util/ShareUtil.java
index 0b2ad1d..60b0196 100644
--- a/app/src/main/java/org/wikipedia/util/ShareUtil.java
+++ b/app/src/main/java/org/wikipedia/util/ShareUtil.java
@@ -16,6 +16,7 @@
import org.wikipedia.BuildConfig;
import org.wikipedia.R;
import org.wikipedia.concurrency.SaneAsyncTask;
+import org.wikipedia.feed.image.FeaturedImageCard;
import org.wikipedia.page.PageTitle;
import org.wikipedia.util.log.L;
@@ -89,6 +90,15 @@
}.execute();
}
+ public static String getFeaturedImageShareSubject(@NonNull Context context,
+ @Nullable
FeaturedImageCard card) {
+ String result =
context.getString(R.string.feed_featured_image_share_subject);
+ if (card != null) {
+ result = result + " | " +
DateUtil.getFeedCardDateString(card.date().baseCalendar());
+ }
+ return result;
+ }
+
public static Intent buildImageShareChooserIntent(Context context, String
subject, String text, Uri uri) {
Intent shareIntent = createImageShareIntent(subject, text, uri);
return Intent.createChooser(shareIntent,
--
To view, visit https://gerrit.wikimedia.org/r/327005
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I74ef3557700018176310097180515664652b13ca
Gerrit-PatchSet: 3
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: Dbrant <dbr...@wikimedia.org>
Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
