From 1808699e89b192f042c98aa7f8d3f904f8de2974 Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Wed, 19 Jun 2024 01:10:30 +0530 Subject: [PATCH] Make new feedback to be added as a new section to the end of the page (#5753) * feedback: add the feedback as a new section at end of the page Addresses feedback on #5542. For auto-archiving of section to work properly on our feedback page, the new sections need to be created at the end of the page rather than at the top. So, adjust the feedback addition logic to make it such that the feedback is appended to the bottom of the page. * Replace lambda with a method reference * feedback: replace edit summary with something more relevant The summary of the feedback page was unhelpful. Make it more helpful by using a more helpful summary that at least mentions the version of the app for which the feedback is posted. * test: try to fix test case related to feedback change --- .../nrw/commons/actions/PageEditClient.kt | 22 ++++ .../nrw/commons/actions/PageEditInterface.kt | 10 ++ .../feedback/FeedbackContentCreator.java | 104 ++++++++++-------- .../navtab/MoreBottomSheetFragment.java | 11 +- .../FeedbackContentCreatorUnitTests.kt | 3 +- .../MoreBottomSheetFragmentUnitTests.kt | 2 +- 6 files changed, 100 insertions(+), 52 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/actions/PageEditClient.kt b/app/src/main/java/fr/free/nrw/commons/actions/PageEditClient.kt index 0036f2f8e..cb153b7cb 100644 --- a/app/src/main/java/fr/free/nrw/commons/actions/PageEditClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/actions/PageEditClient.kt @@ -78,6 +78,28 @@ class PageEditClient( } + /** + * Appends a new section to the wiki page + * @param pageTitle Title of the page to edit + * @param sectionTitle Title of the new section that needs to be created + * @param sectionText The page content that is to be added to the section + * @param summary Edit summary + * @return whether the edit was successful + */ + fun createNewSection(pageTitle: String, sectionTitle: String, sectionText: String, summary: String): Observable { + return try { + pageEditInterface.postNewSection(pageTitle, summary, sectionTitle, sectionText, csrfTokenClient.getTokenBlocking()) + .map { editResponse -> editResponse.edit()!!.editSucceeded() } + } catch (throwable: Throwable) { + if (throwable is InvalidLoginTokenException) { + throw throwable + } else { + Observable.just(false) + } + } + } + + /** * Set new labels to Wikibase server of commons * @param summary Edit summary diff --git a/app/src/main/java/fr/free/nrw/commons/actions/PageEditInterface.kt b/app/src/main/java/fr/free/nrw/commons/actions/PageEditInterface.kt index 56f0bf610..070aaaa64 100644 --- a/app/src/main/java/fr/free/nrw/commons/actions/PageEditInterface.kt +++ b/app/src/main/java/fr/free/nrw/commons/actions/PageEditInterface.kt @@ -74,6 +74,16 @@ interface PageEditInterface { @Field("token") token: String ): Observable + @FormUrlEncoded + @Headers("Cache-Control: no-cache") + @POST(MW_API_PREFIX + "action=edit§ion=new") + fun postNewSection( + @Field("title") title: String, + @Field("summary") summary: String, + @Field("sectiontitle") sectionTitle: String, + @Field("text") sectionText: String, + @Field("token") token: String + ): Observable @FormUrlEncoded @Headers("Cache-Control: no-cache") diff --git a/app/src/main/java/fr/free/nrw/commons/feedback/FeedbackContentCreator.java b/app/src/main/java/fr/free/nrw/commons/feedback/FeedbackContentCreator.java index 5fe0cac4e..839858b3e 100644 --- a/app/src/main/java/fr/free/nrw/commons/feedback/FeedbackContentCreator.java +++ b/app/src/main/java/fr/free/nrw/commons/feedback/FeedbackContentCreator.java @@ -12,7 +12,8 @@ import java.util.Locale; * from feedback information */ public class FeedbackContentCreator { - private StringBuilder stringBuilder; + private StringBuilder sectionTextBuilder; + private StringBuilder sectionTitleBuilder; private Feedback feedback; private Context context; @@ -28,71 +29,80 @@ public class FeedbackContentCreator { public void init() { // Localization is not needed here, because this ends up on a page where developers read the feedback, so English is the most convenient. - stringBuilder = new StringBuilder(); - stringBuilder.append("== "); - stringBuilder.append("Feedback from "); - stringBuilder.append(AccountUtil.getUserName(context)); - stringBuilder.append(" for version "); - stringBuilder.append(feedback.getVersion()); - stringBuilder.append(" =="); - stringBuilder.append("\n"); - stringBuilder.append(feedback.getTitle()); - stringBuilder.append("\n"); - stringBuilder.append("\n"); + /* + * Construct the feedback section title + */ + sectionTitleBuilder = new StringBuilder(); + sectionTitleBuilder.append("Feedback from "); + sectionTitleBuilder.append(AccountUtil.getUserName(context)); + sectionTitleBuilder.append(" for version "); + sectionTitleBuilder.append(feedback.getVersion()); + + /* + * Construct the feedback section text + */ + sectionTextBuilder = new StringBuilder(); + sectionTextBuilder.append("\n"); + sectionTextBuilder.append(feedback.getTitle()); + sectionTextBuilder.append("\n"); + sectionTextBuilder.append("\n"); if (feedback.getApiLevel() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.api_level)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getApiLevel()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getApiLevel()); + sectionTextBuilder.append("\n"); } if (feedback.getAndroidVersion() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.android_version)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getAndroidVersion()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getAndroidVersion()); + sectionTextBuilder.append("\n"); } if (feedback.getDeviceManufacturer() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.device_manufacturer)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getDeviceManufacturer()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getDeviceManufacturer()); + sectionTextBuilder.append("\n"); } if (feedback.getDeviceModel() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.device_model)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getDeviceModel()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getDeviceModel()); + sectionTextBuilder.append("\n"); } if (feedback.getDevice() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.device_name)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getDevice()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getDevice()); + sectionTextBuilder.append("\n"); } if (feedback.getNetworkType() != null) { - stringBuilder.append("* "); - stringBuilder.append(LangCodeUtils.getLocalizedResources(context, + sectionTextBuilder.append("* "); + sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context, Locale.ENGLISH).getString(R.string.network_type)); - stringBuilder.append(": "); - stringBuilder.append(feedback.getNetworkType()); - stringBuilder.append("\n"); + sectionTextBuilder.append(": "); + sectionTextBuilder.append(feedback.getNetworkType()); + sectionTextBuilder.append("\n"); } - stringBuilder.append("~~~~"); - stringBuilder.append("\n"); + sectionTextBuilder.append("~~~~"); + sectionTextBuilder.append("\n"); } - @Override - public String toString() { - return stringBuilder.toString(); + public String getSectionText() { + return sectionTextBuilder.toString(); + } + + public String getSectionTitle() { + return sectionTitleBuilder.toString(); } } diff --git a/app/src/main/java/fr/free/nrw/commons/navtab/MoreBottomSheetFragment.java b/app/src/main/java/fr/free/nrw/commons/navtab/MoreBottomSheetFragment.java index 760593281..0bd8333e3 100644 --- a/app/src/main/java/fr/free/nrw/commons/navtab/MoreBottomSheetFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/navtab/MoreBottomSheetFragment.java @@ -153,9 +153,14 @@ public class MoreBottomSheetFragment extends BottomSheetDialogFragment { void uploadFeedback(final Feedback feedback) { final FeedbackContentCreator feedbackContentCreator = new FeedbackContentCreator(getContext(), feedback); - Single single = - pageEditClient.prependEdit("Commons:Mobile_app/Feedback", feedbackContentCreator.toString(), "Summary") - .flatMapSingle(result -> Single.just(result)) + final Single single = + pageEditClient.createNewSection( + "Commons:Mobile_app/Feedback", + feedbackContentCreator.getSectionTitle(), + feedbackContentCreator.getSectionText(), + "New feedback on version " + feedback.getVersion() + " of the app" + ) + .flatMapSingle(Single::just) .firstOrError(); Single.defer((Callable>) () -> single) diff --git a/app/src/test/kotlin/fr/free/nrw/commons/feedback/FeedbackContentCreatorUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/feedback/FeedbackContentCreatorUnitTests.kt index a2fea95e1..a238d8f7a 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/feedback/FeedbackContentCreatorUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/feedback/FeedbackContentCreatorUnitTests.kt @@ -34,7 +34,8 @@ class FeedbackContentCreatorUnitTests { fun testToString() { feedback = Feedback("123", "apiLevel", "title", "androidVersion", "deviceModel", "mfg", "deviceName", "wifi") creator = FeedbackContentCreator(context, feedback) - Assert.assertNotNull(creator.toString()) + Assert.assertNotNull(creator.getSectionText()) + Assert.assertNotNull(creator.getSectionTitle()) } } \ No newline at end of file diff --git a/app/src/test/kotlin/fr/free/nrw/commons/navtab/MoreBottomSheetFragmentUnitTests.kt b/app/src/test/kotlin/fr/free/nrw/commons/navtab/MoreBottomSheetFragmentUnitTests.kt index f9606b8d9..a5faed37e 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/navtab/MoreBottomSheetFragmentUnitTests.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/navtab/MoreBottomSheetFragmentUnitTests.kt @@ -119,7 +119,7 @@ class MoreBottomSheetFragmentUnitTests { val feedback = mock(Feedback::class.java) val observable: Observable = Observable.just(false) val observable2: Observable = Observable.just(true) - doReturn(observable, observable2).`when`(pageEditClient).prependEdit(anyString(), anyString(), anyString()) + doReturn(observable, observable2).`when`(pageEditClient).createNewSection(anyString(), anyString(), anyString(), anyString()) fragment.uploadFeedback(feedback) }