Skip to content

Commit 1808699

Browse files
authored
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
1 parent 0e39d93 commit 1808699

File tree

6 files changed

+100
-52
lines changed

6 files changed

+100
-52
lines changed

app/src/main/java/fr/free/nrw/commons/actions/PageEditClient.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,28 @@ class PageEditClient(
7878
}
7979

8080

81+
/**
82+
* Appends a new section to the wiki page
83+
* @param pageTitle Title of the page to edit
84+
* @param sectionTitle Title of the new section that needs to be created
85+
* @param sectionText The page content that is to be added to the section
86+
* @param summary Edit summary
87+
* @return whether the edit was successful
88+
*/
89+
fun createNewSection(pageTitle: String, sectionTitle: String, sectionText: String, summary: String): Observable<Boolean> {
90+
return try {
91+
pageEditInterface.postNewSection(pageTitle, summary, sectionTitle, sectionText, csrfTokenClient.getTokenBlocking())
92+
.map { editResponse -> editResponse.edit()!!.editSucceeded() }
93+
} catch (throwable: Throwable) {
94+
if (throwable is InvalidLoginTokenException) {
95+
throw throwable
96+
} else {
97+
Observable.just(false)
98+
}
99+
}
100+
}
101+
102+
81103
/**
82104
* Set new labels to Wikibase server of commons
83105
* @param summary Edit summary

app/src/main/java/fr/free/nrw/commons/actions/PageEditInterface.kt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ interface PageEditInterface {
7474
@Field("token") token: String
7575
): Observable<Edit>
7676

77+
@FormUrlEncoded
78+
@Headers("Cache-Control: no-cache")
79+
@POST(MW_API_PREFIX + "action=edit&section=new")
80+
fun postNewSection(
81+
@Field("title") title: String,
82+
@Field("summary") summary: String,
83+
@Field("sectiontitle") sectionTitle: String,
84+
@Field("text") sectionText: String,
85+
@Field("token") token: String
86+
): Observable<Edit>
7787

7888
@FormUrlEncoded
7989
@Headers("Cache-Control: no-cache")

app/src/main/java/fr/free/nrw/commons/feedback/FeedbackContentCreator.java

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
* from feedback information
1313
*/
1414
public class FeedbackContentCreator {
15-
private StringBuilder stringBuilder;
15+
private StringBuilder sectionTextBuilder;
16+
private StringBuilder sectionTitleBuilder;
1617
private Feedback feedback;
1718
private Context context;
1819

@@ -28,71 +29,80 @@ public FeedbackContentCreator(Context context, Feedback feedback) {
2829
public void init() {
2930
// Localization is not needed here, because this ends up on a page where developers read the feedback, so English is the most convenient.
3031

31-
stringBuilder = new StringBuilder();
32-
stringBuilder.append("== ");
33-
stringBuilder.append("Feedback from ");
34-
stringBuilder.append(AccountUtil.getUserName(context));
35-
stringBuilder.append(" for version ");
36-
stringBuilder.append(feedback.getVersion());
37-
stringBuilder.append(" ==");
38-
stringBuilder.append("\n");
39-
stringBuilder.append(feedback.getTitle());
40-
stringBuilder.append("\n");
41-
stringBuilder.append("\n");
32+
/*
33+
* Construct the feedback section title
34+
*/
35+
sectionTitleBuilder = new StringBuilder();
36+
sectionTitleBuilder.append("Feedback from ");
37+
sectionTitleBuilder.append(AccountUtil.getUserName(context));
38+
sectionTitleBuilder.append(" for version ");
39+
sectionTitleBuilder.append(feedback.getVersion());
40+
41+
/*
42+
* Construct the feedback section text
43+
*/
44+
sectionTextBuilder = new StringBuilder();
45+
sectionTextBuilder.append("\n");
46+
sectionTextBuilder.append(feedback.getTitle());
47+
sectionTextBuilder.append("\n");
48+
sectionTextBuilder.append("\n");
4249
if (feedback.getApiLevel() != null) {
43-
stringBuilder.append("* ");
44-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
50+
sectionTextBuilder.append("* ");
51+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
4552
Locale.ENGLISH).getString(R.string.api_level));
46-
stringBuilder.append(": ");
47-
stringBuilder.append(feedback.getApiLevel());
48-
stringBuilder.append("\n");
53+
sectionTextBuilder.append(": ");
54+
sectionTextBuilder.append(feedback.getApiLevel());
55+
sectionTextBuilder.append("\n");
4956
}
5057
if (feedback.getAndroidVersion() != null) {
51-
stringBuilder.append("* ");
52-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
58+
sectionTextBuilder.append("* ");
59+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
5360
Locale.ENGLISH).getString(R.string.android_version));
54-
stringBuilder.append(": ");
55-
stringBuilder.append(feedback.getAndroidVersion());
56-
stringBuilder.append("\n");
61+
sectionTextBuilder.append(": ");
62+
sectionTextBuilder.append(feedback.getAndroidVersion());
63+
sectionTextBuilder.append("\n");
5764
}
5865
if (feedback.getDeviceManufacturer() != null) {
59-
stringBuilder.append("* ");
60-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
66+
sectionTextBuilder.append("* ");
67+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
6168
Locale.ENGLISH).getString(R.string.device_manufacturer));
62-
stringBuilder.append(": ");
63-
stringBuilder.append(feedback.getDeviceManufacturer());
64-
stringBuilder.append("\n");
69+
sectionTextBuilder.append(": ");
70+
sectionTextBuilder.append(feedback.getDeviceManufacturer());
71+
sectionTextBuilder.append("\n");
6572
}
6673
if (feedback.getDeviceModel() != null) {
67-
stringBuilder.append("* ");
68-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
74+
sectionTextBuilder.append("* ");
75+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
6976
Locale.ENGLISH).getString(R.string.device_model));
70-
stringBuilder.append(": ");
71-
stringBuilder.append(feedback.getDeviceModel());
72-
stringBuilder.append("\n");
77+
sectionTextBuilder.append(": ");
78+
sectionTextBuilder.append(feedback.getDeviceModel());
79+
sectionTextBuilder.append("\n");
7380
}
7481
if (feedback.getDevice() != null) {
75-
stringBuilder.append("* ");
76-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
82+
sectionTextBuilder.append("* ");
83+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
7784
Locale.ENGLISH).getString(R.string.device_name));
78-
stringBuilder.append(": ");
79-
stringBuilder.append(feedback.getDevice());
80-
stringBuilder.append("\n");
85+
sectionTextBuilder.append(": ");
86+
sectionTextBuilder.append(feedback.getDevice());
87+
sectionTextBuilder.append("\n");
8188
}
8289
if (feedback.getNetworkType() != null) {
83-
stringBuilder.append("* ");
84-
stringBuilder.append(LangCodeUtils.getLocalizedResources(context,
90+
sectionTextBuilder.append("* ");
91+
sectionTextBuilder.append(LangCodeUtils.getLocalizedResources(context,
8592
Locale.ENGLISH).getString(R.string.network_type));
86-
stringBuilder.append(": ");
87-
stringBuilder.append(feedback.getNetworkType());
88-
stringBuilder.append("\n");
93+
sectionTextBuilder.append(": ");
94+
sectionTextBuilder.append(feedback.getNetworkType());
95+
sectionTextBuilder.append("\n");
8996
}
90-
stringBuilder.append("~~~~");
91-
stringBuilder.append("\n");
97+
sectionTextBuilder.append("~~~~");
98+
sectionTextBuilder.append("\n");
99+
}
100+
101+
public String getSectionText() {
102+
return sectionTextBuilder.toString();
92103
}
93104

94-
@Override
95-
public String toString() {
96-
return stringBuilder.toString();
105+
public String getSectionTitle() {
106+
return sectionTitleBuilder.toString();
97107
}
98108
}

app/src/main/java/fr/free/nrw/commons/navtab/MoreBottomSheetFragment.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,14 @@ private void showFeedbackDialog() {
153153
void uploadFeedback(final Feedback feedback) {
154154
final FeedbackContentCreator feedbackContentCreator = new FeedbackContentCreator(getContext(), feedback);
155155

156-
Single<Boolean> single =
157-
pageEditClient.prependEdit("Commons:Mobile_app/Feedback", feedbackContentCreator.toString(), "Summary")
158-
.flatMapSingle(result -> Single.just(result))
156+
final Single<Boolean> single =
157+
pageEditClient.createNewSection(
158+
"Commons:Mobile_app/Feedback",
159+
feedbackContentCreator.getSectionTitle(),
160+
feedbackContentCreator.getSectionText(),
161+
"New feedback on version " + feedback.getVersion() + " of the app"
162+
)
163+
.flatMapSingle(Single::just)
159164
.firstOrError();
160165

161166
Single.defer((Callable<SingleSource<Boolean>>) () -> single)

app/src/test/kotlin/fr/free/nrw/commons/feedback/FeedbackContentCreatorUnitTests.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ class FeedbackContentCreatorUnitTests {
3434
fun testToString() {
3535
feedback = Feedback("123", "apiLevel", "title", "androidVersion", "deviceModel", "mfg", "deviceName", "wifi")
3636
creator = FeedbackContentCreator(context, feedback)
37-
Assert.assertNotNull(creator.toString())
37+
Assert.assertNotNull(creator.getSectionText())
38+
Assert.assertNotNull(creator.getSectionTitle())
3839
}
3940

4041
}

app/src/test/kotlin/fr/free/nrw/commons/navtab/MoreBottomSheetFragmentUnitTests.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class MoreBottomSheetFragmentUnitTests {
119119
val feedback = mock(Feedback::class.java)
120120
val observable: Observable<Boolean> = Observable.just(false)
121121
val observable2: Observable<Boolean> = Observable.just(true)
122-
doReturn(observable, observable2).`when`(pageEditClient).prependEdit(anyString(), anyString(), anyString())
122+
doReturn(observable, observable2).`when`(pageEditClient).createNewSection(anyString(), anyString(), anyString(), anyString())
123123
fragment.uploadFeedback(feedback)
124124
}
125125

0 commit comments

Comments
 (0)