[MediaWiki-commits] [Gerrit] apps...wikipedia[master]: Hygiene: Clean up misuse of PageTitles in GalleryActivity

2017-01-23 Thread jenkins-bot (Code Review)
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 == 

[MediaWiki-commits] [Gerrit] apps...wikipedia[master]: Hygiene: Clean up misuse of PageTitles in GalleryActivity

2016-12-13 Thread Mholloway (Code Review)
Mholloway has uploaded a new change for review. ( 
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.

Bug: T152980
Change-Id: I74ef3557700018176310097180515664652b13ca
---
M app/src/main/java/org/wikipedia/analytics/GalleryFunnel.java
M app/src/main/java/org/wikipedia/main/MainFragment.java
M app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
M app/src/main/java/org/wikipedia/page/gallery/GalleryItemFragment.java
M app/src/main/java/org/wikipedia/util/ShareUtil.java
5 files changed, 42 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia 
refs/changes/05/327005/1

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/main/MainFragment.java 
b/app/src/main/java/org/wikipedia/main/MainFragment.java
index d9ac607..3cee59d 100644
--- a/app/src/main/java/org/wikipedia/main/MainFragment.java
+++ b/app/src/main/java/org/wikipedia/main/MainFragment.java
@@ -54,7 +54,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;
@@ -250,8 +249,7 @@
 ShareUtil.shareImage(getContext(),
 bitmap,
 new File(thumbUrl).getName(),
-
getString(R.string.feed_featured_image_share_subject) + " | "
-+ 
DateUtil.getFeedCardDateString(card.date().baseCalendar()),
+
ShareUtil.getFeaturedImageShareSubject(getContext(), card),
 fullSizeUrl);
 } else {
 FeedbackUtil.showMessage(MainFragment.this, 
getString(R.string.gallery_share_error, card.baseImage().title()));
@@ -270,8 +268,8 @@
 }
 
 @Override public void onFeaturedImageSelected(FeaturedImageCard card) {
-startActivityForResult(GalleryActivity.newIntent(getActivity(), 
card.baseImage(),
-card.filename(), card.wikiSite(), 
GalleryFunnel.SOURCE_FEED_FEATURED_IMAGE),
+startActivityForResult(GalleryActivity.newIntent(getActivity(), 
card.filename(),
+card.wikiSite(), GalleryFunnel.SOURCE_FEED_FEATURED_IMAGE, 
card.baseImage()),
 Constants.ACTIVITY_REQUEST_GALLERY);
 }
 
diff --git a/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java 
b/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
index c7e3ce9..8c9deaf 100644
--- a/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
+++ b/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
@@ -69,8 +69,6 @@
 public static final String EXTRA_SOURCE = "source";
 public static final String EXTRA_FEATURED_IMAGE = "card";
 
-private static final String FEED_FEATURED_IMAGE_TITLE = 
"FeedFeaturedImage";
-
 @NonNull private WikipediaApp app = WikipediaApp.getInstance();
 @Nullable private PageTitle pageTitle;
 @Nullable private Page page;
@@ -139,21 +137,26 @@
 };
 
 @NonNull
-public static Intent newIntent(@NonNull Context