From cf73e2862364c07e10c1bac63b70f625827a281a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20Mac=20Gillicuddy?= Date: Wed, 8 Apr 2020 17:39:41 +0100 Subject: [PATCH 1/9] #3624 DateTimeFormat wrong - match pattern returned from servers (#3625) --- .../nrw/commons/utils/CommonsDateUtil.java | 9 +++++---- .../nrw/commons/utils/CommonsDateUtilTest.kt | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 app/src/test/kotlin/fr/free/nrw/commons/utils/CommonsDateUtilTest.kt diff --git a/app/src/main/java/fr/free/nrw/commons/utils/CommonsDateUtil.java b/app/src/main/java/fr/free/nrw/commons/utils/CommonsDateUtil.java index fc3b9ff2c..d03dd8fe4 100644 --- a/app/src/main/java/fr/free/nrw/commons/utils/CommonsDateUtil.java +++ b/app/src/main/java/fr/free/nrw/commons/utils/CommonsDateUtil.java @@ -25,9 +25,10 @@ public class CommonsDateUtil { * Gets the timestamp pattern for a date * @return timestamp */ - public static SimpleDateFormat getIso8601DateFormatTimestamp() { - SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss", Locale.ROOT); - simpleDateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); - return simpleDateFormat; + public static SimpleDateFormat getIso8601DateFormatTimestamp() { + final SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssX", + Locale.ROOT); + simpleDateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); + return simpleDateFormat; } } diff --git a/app/src/test/kotlin/fr/free/nrw/commons/utils/CommonsDateUtilTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/utils/CommonsDateUtilTest.kt new file mode 100644 index 000000000..9056cb755 --- /dev/null +++ b/app/src/test/kotlin/fr/free/nrw/commons/utils/CommonsDateUtilTest.kt @@ -0,0 +1,20 @@ +package fr.free.nrw.commons.utils + +import org.hamcrest.core.IsEqual.equalTo +import org.junit.Assert.assertThat +import org.junit.Test + +class CommonsDateUtilTest { + + @Test + fun `Iso8601DateFormatTimestamp parses legal date`() { + val iso8601DateFormatTimestamp = CommonsDateUtil + .getIso8601DateFormatTimestamp() + val parsedDate = iso8601DateFormatTimestamp + .parse("2020-04-07T14:21:57Z") + assertThat( + "2020-04-07T14:21:57Z", + equalTo(iso8601DateFormatTimestamp.format(parsedDate)) + ) + } +} From c961099013a19b91c46763216331795c36957b5d Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Fri, 10 Apr 2020 09:38:51 +0000 Subject: [PATCH 2/9] Revert "Fixes: #3179 Make category search non case-sensitive (#3326)" (#3636) Simply lower casing the name of the category sent to the server doesn't result in the server doing a case insensitive category search. In fact, it reduces the category search space as only categories that has a lower case character is searched even if the search text contains upper case characters. The test case did not catch this issue as the first character of the title is case insensitive[1]. So, revert the changes done in commit afdeaae0757d4d7773921169eb65d3ba9916834a. See further disucssion in the issue thread of #3179 starting from [2]. [1]: https://www.mediawiki.org/wiki/Manual:Page_title [2]: https://github.com/commons-app/apps-android-commons/issues/3179#issuecomment-605462140 --- .../nrw/commons/category/CategoriesModel.java | 6 +-- .../nrw/commons/category/CategoryClient.java | 2 +- .../nrw/commons/category/CategoryItem.java | 37 ++++++++++++++++++- .../commons/category/CategoriesModelTest.kt | 25 ------------- .../commons/category/CategoryClientTest.kt | 2 - 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java index caa66e433..e2030ee9a 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoriesModel.java @@ -123,9 +123,8 @@ public class CategoriesModel{ } //otherwise, search API for matching categories - //term passed as lower case to make search case-insensitive(taking only lower case for everything) return categoryClient - .searchCategoriesForPrefix(term.toLowerCase(), SEARCH_CATS_LIMIT) + .searchCategoriesForPrefix(term, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); } @@ -184,12 +183,11 @@ public class CategoriesModel{ /** * Return category for single title - * title is converted to lower case to make search case-insensitive * @param title * @return */ private Observable getTitleCategories(String title) { - return categoryClient.searchCategories(title.toLowerCase(), SEARCH_CATS_LIMIT) + return categoryClient.searchCategories(title, SEARCH_CATS_LIMIT) .map(name -> new CategoryItem(name, false)); } diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java index 7a4d1718b..329c3635a 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryClient.java @@ -55,7 +55,7 @@ public class CategoryClient { /** * Searches for categories starting with the specified string. - * + * * @param prefix The prefix to be searched * @param itemLimit How many results are returned * @param offset Starts returning items from the nth result. If offset is 9, the response starts with the 9th item of the search result diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java index c8f2a2713..bb89a11c4 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryItem.java @@ -3,6 +3,10 @@ package fr.free.nrw.commons.category; import android.os.Parcel; import android.os.Parcelable; +/** + * Represents a Category Item. + * Implemented as Parcelable so that its object could be parsed between activity components. + */ public class CategoryItem implements Parcelable { private final String name; private boolean selected; @@ -24,28 +28,53 @@ public class CategoryItem implements Parcelable { this.selected = selected; } + /** + * Reads from the received Parcel + * @param in + */ private CategoryItem(Parcel in) { name = in.readString(); selected = in.readInt() == 1; } + /** + * Gets Name + * @return + */ public String getName() { return name; } + /** + * Checks if that Category Item has been selected. + * @return + */ public boolean isSelected() { return selected; } + /** + * Selects the Category Item. + * @param selected + */ public void setSelected(boolean selected) { this.selected = selected; } + /** + * Used by Parcelable + * @return + */ @Override public int describeContents() { return 0; } + /** + * Writes to the received Parcel + * @param parcel + * @param flags + */ @Override public void writeToParcel(Parcel parcel, int flags) { parcel.writeString(name); @@ -67,13 +96,19 @@ public class CategoryItem implements Parcelable { } + /** + * Returns hash code for current object + */ @Override public int hashCode() { return name.hashCode(); } + /** + * Return String form of current object + */ @Override public String toString() { return "CategoryItem: '" + name + '\''; } -} \ No newline at end of file +} diff --git a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt index 8cd7e292c..fea98f450 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoriesModelTest.kt @@ -17,9 +17,6 @@ class CategoriesModelTest { @Mock internal var categoryInterface: CategoryInterface? = null - @Mock - internal var categoryItem: CategoryItem? = null - @Spy internal lateinit var gson: Gson @@ -43,28 +40,6 @@ class CategoriesModelTest { MockitoAnnotations.initMocks(this) } - // Test Case for verifying that Categories search (MW api calls) are case-insensitive - @Test - fun searchAllFoundCaseTest() { - val mwQueryPage = Mockito.mock(MwQueryPage::class.java) - Mockito.`when`(mwQueryPage.title()).thenReturn("Category:Test") - val mwQueryResult = Mockito.mock(MwQueryResult::class.java) - Mockito.`when`(mwQueryResult.pages()).thenReturn(listOf(mwQueryPage)) - val mockResponse = Mockito.mock(MwQueryResponse::class.java) - Mockito.`when`(mockResponse.query()).thenReturn(mwQueryResult) - val categoriesModel: CategoriesModel = CategoriesModel(categoryClient,null,null) - - Mockito.`when`(categoryInterface!!.searchCategoriesForPrefix(ArgumentMatchers.anyString(), ArgumentMatchers.anyInt(), ArgumentMatchers.anyInt())) - .thenReturn(Observable.just(mockResponse)) - - // Checking if both return "Test" - val actualCategoryName = categoriesModel!!.searchAll("tes",null).blockingFirst() - assertEquals("Test", actualCategoryName.getName()) - - val actualCategoryNameCaps = categoriesModel!!.searchAll("Tes",null).blockingFirst() - assertEquals("Test", actualCategoryNameCaps.getName()) - } - /** * For testing the substring search algorithm for Categories search * To be more precise it tests the In Between substring( ex: searching `atte` diff --git a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt index a668e5704..7c26cc323 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/category/CategoryClientTest.kt @@ -58,7 +58,6 @@ class CategoryClientTest { { fail("SearchCategories returned element when it shouldn't have.") }, { s -> throw s }) } - @Test fun searchCategoriesForPrefixFound() { val mwQueryPage = Mockito.mock(MwQueryPage::class.java) @@ -93,7 +92,6 @@ class CategoryClientTest { { fail("SearchCategories returned element when it shouldn't have.") }, { s -> throw s }) } - @Test fun getParentCategoryListFound() { val mwQueryPage = Mockito.mock(MwQueryPage::class.java) From 05a9aa857551cf26b55cff083e54fff68c4bc73c Mon Sep 17 00:00:00 2001 From: Ashish Kumar Date: Fri, 17 Apr 2020 12:29:30 +0530 Subject: [PATCH 3/9] Bugfix/security exception (#3627) * Fixes #3626 * Check is file is actually created before writing to file, file picker android * Handle Security exception --- .../java/fr/free/nrw/commons/filepicker/FilePicker.java | 4 ++-- .../java/fr/free/nrw/commons/filepicker/PickedFiles.java | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java b/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java index 5d168d6b1..698e2d51f 100644 --- a/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java +++ b/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java @@ -166,7 +166,7 @@ public class FilePicker implements Constants { public static List handleExternalImagesPicked(Intent data, Activity activity) { try { return getFilesFromGalleryPictures(data, activity); - } catch (IOException e) { + } catch (IOException | SecurityException e) { e.printStackTrace(); } return new ArrayList<>(); @@ -207,7 +207,7 @@ public class FilePicker implements Constants { } } - private static List getFilesFromGalleryPictures(Intent data, Activity activity) throws IOException { + private static List getFilesFromGalleryPictures(Intent data, Activity activity) throws IOException, SecurityException { List files = new ArrayList<>(); ClipData clipData = data.getClipData(); if (clipData == null) { diff --git a/app/src/main/java/fr/free/nrw/commons/filepicker/PickedFiles.java b/app/src/main/java/fr/free/nrw/commons/filepicker/PickedFiles.java index 86da1326b..01e68c940 100644 --- a/app/src/main/java/fr/free/nrw/commons/filepicker/PickedFiles.java +++ b/app/src/main/java/fr/free/nrw/commons/filepicker/PickedFiles.java @@ -104,12 +104,15 @@ class PickedFiles implements Constants { }); } - static UploadableFile pickedExistingPicture(@NonNull Context context, Uri photoUri) throws IOException { + static UploadableFile pickedExistingPicture(@NonNull Context context, Uri photoUri) throws IOException, SecurityException {// SecurityException for those file providers who share URI but forget to grant necessary permissions InputStream pictureInputStream = context.getContentResolver().openInputStream(photoUri); File directory = tempImageDirectory(context); File photoFile = new File(directory, UUID.randomUUID().toString() + "." + getMimeType(context, photoUri)); - photoFile.createNewFile(); - writeToFile(pictureInputStream, photoFile); + if (photoFile.createNewFile()) { + writeToFile(pictureInputStream, photoFile); + } else { + throw new IOException("could not create photoFile to write upon"); + } return new UploadableFile(photoUri, photoFile); } From d7c2480174db15bd834872d7b669b1acc6ca85ea Mon Sep 17 00:00:00 2001 From: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com> Date: Sun, 19 Apr 2020 18:24:17 -0400 Subject: [PATCH 4/9] Fixes #3436 and #2881: Media Detail design Overhaul (#3505) * ic_map_dark_24dp: map icon for white background * ic_info_outline_dark_24dp: info icon for dark background * MediaDetailFragment: update the spacer as per image aspect ratio * fragment_media_detail: design overhaul * fragment_media_detail: remove redundant background color statements * make requested changes * add dark mode support * minor ui tweak * white map icon in dark mode * make rquested changes * make requested changes to layout * fix misalignment of category list * subtle amendments * convert comments to javadocs * minor amendments * minor changes * add styles for media detail * Media detail fragment refactored * make suggested changes * minor name fix * fix the delete button border --- .../commons/media/MediaDetailFragment.java | 13 +- .../res/drawable/bg_copy_wikitext_button.xml | 9 + .../main/res/drawable/bg_delete_button.xml | 5 +- .../drawable/ic_info_outline_dark_24dp.xml | 9 + .../main/res/drawable/ic_map_dark_24dp.xml | 9 + .../main/res/layout/detail_category_item.xml | 10 +- .../main/res/layout/fragment_media_detail.xml | 265 +++++++----------- app/src/main/res/values/attrs.xml | 6 + app/src/main/res/values/colors.xml | 3 +- app/src/main/res/values/styles.xml | 43 +++ 10 files changed, 194 insertions(+), 178 deletions(-) create mode 100644 app/src/main/res/drawable/bg_copy_wikitext_button.xml create mode 100644 app/src/main/res/drawable/ic_info_outline_dark_24dp.xml create mode 100644 app/src/main/res/drawable/ic_map_dark_24dp.xml diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java index 2349b6236..689e62395 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java @@ -105,6 +105,8 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { @BindView(R.id.mediaDetailImageView) SimpleDraweeView image; + @BindView(R.id.mediaDetailImageViewSpacer) + LinearLayout imageSpacer; @BindView(R.id.mediaDetailTitle) TextView title; @BindView(R.id.mediaDetailDesc) @@ -205,7 +207,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { return view; } - @OnClick(R.id.mediaDetailImageView) + @OnClick(R.id.mediaDetailImageViewSpacer) public void launchZoomActivity(View view) { Context ctx = view.getContext(); ctx.startActivity( @@ -241,12 +243,21 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { compositeDisposable.add(disposable); } + /** + * The imageSpacer is Basically a transparent overlay for the SimpleDraweeView + * which holds the image to be displayed( moreover this image is out of + * the scroll view ) + * @param imageInfo used to calculate height of the ImageSpacer + */ private void updateAspectRatio(ImageInfo imageInfo) { if (imageInfo != null) { int finalHeight = (scrollView.getWidth()*imageInfo.getHeight()) / imageInfo.getWidth(); ViewGroup.LayoutParams params = image.getLayoutParams(); + ViewGroup.LayoutParams spacerParams = imageSpacer.getLayoutParams(); params.height = finalHeight; + spacerParams.height = finalHeight; image.setLayoutParams(params); + imageSpacer.setLayoutParams(spacerParams); } } diff --git a/app/src/main/res/drawable/bg_copy_wikitext_button.xml b/app/src/main/res/drawable/bg_copy_wikitext_button.xml new file mode 100644 index 000000000..a69e1073a --- /dev/null +++ b/app/src/main/res/drawable/bg_copy_wikitext_button.xml @@ -0,0 +1,9 @@ + + + + + \ No newline at end of file diff --git a/app/src/main/res/drawable/bg_delete_button.xml b/app/src/main/res/drawable/bg_delete_button.xml index 819986f5b..2eab78e89 100644 --- a/app/src/main/res/drawable/bg_delete_button.xml +++ b/app/src/main/res/drawable/bg_delete_button.xml @@ -9,12 +9,9 @@ + android:color="?attr/mediaDetailNominationBackground"/> - diff --git a/app/src/main/res/drawable/ic_info_outline_dark_24dp.xml b/app/src/main/res/drawable/ic_info_outline_dark_24dp.xml new file mode 100644 index 000000000..79fc47261 --- /dev/null +++ b/app/src/main/res/drawable/ic_info_outline_dark_24dp.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/drawable/ic_map_dark_24dp.xml b/app/src/main/res/drawable/ic_map_dark_24dp.xml new file mode 100644 index 000000000..2063dfe95 --- /dev/null +++ b/app/src/main/res/drawable/ic_map_dark_24dp.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/layout/detail_category_item.xml b/app/src/main/res/layout/detail_category_item.xml index 02fb6c80f..181fdb1e0 100644 --- a/app/src/main/res/layout/detail_category_item.xml +++ b/app/src/main/res/layout/detail_category_item.xml @@ -10,15 +10,15 @@ android:id="@+id/mediaDetailCategoryItemText" android:layout_width="match_parent" android:layout_height="wrap_content" - android:background="?attr/subBackground" + android:background="?attr/mainBackground" android:foreground="?attr/selectableItemBackground" android:gravity="center_vertical" android:minHeight="@dimen/overflow_button_dimen" - android:padding="@dimen/quarter_standard_height" - android:textColor="@android:color/white" + android:padding="@dimen/small_gap" + android:textColor="?attr/mediaDetailsText" android:textSize="@dimen/description_text_size" - app:drawablePadding="@dimen/small_gap" - app:drawableStart="@drawable/ic_info_outline_24dp" + app:drawablePadding="@dimen/tiny_gap" + app:drawableStart="?attr/iconInfo24" /> diff --git a/app/src/main/res/layout/fragment_media_detail.xml b/app/src/main/res/layout/fragment_media_detail.xml index 1cdcccaa5..1e238f377 100644 --- a/app/src/main/res/layout/fragment_media_detail.xml +++ b/app/src/main/res/layout/fragment_media_detail.xml @@ -19,11 +19,10 @@ /> + android:layout_height="@dimen/dimen_250" + app:actualImageScaleType="none" /> - + android:orientation="vertical" + android:background="@android:color/transparent" + android:id="@+id/mediaDetailImageViewSpacer" + /> + android:background="?attr/mainBackground" + android:orientation="vertical"> + android:background="@color/primaryDarkColor" + android:orientation="horizontal" + android:padding="@dimen/quarter_standard_height"> + + style="@style/MediaDetailTextLabelTitle" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_title" /> - - + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_author" /> - - + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_description" /> - + android:layout_height="@dimen/tiny_gap"/> + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_license" /> - - + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_coordinates" /> - - + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/detail_panel_cats_label" /> - - + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_uploaded_date" /> - - + + - - + android:orientation="horizontal"> + style="@style/MediaDetailTextLabelGeneric" + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" + android:text="@string/media_detail_discussion" /> + android:layout_width="@dimen/widget_margin" + android:layout_height="match_parent" />