Skip to content

Commit 9ba7e72

Browse files
iMattProiMattPro
committed
Reword the notifications and use the avatar of the status changer
Signed-off-by: Matt Friedman <maf675@gmail.com>
1 parent 5537380 commit 9ba7e72

5 files changed

Lines changed: 82 additions & 34 deletions

File tree

‎factory/idea.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public function set_status($idea_id, $status)
7676
$this->notification_manager->$method(ext::NOTIFICATION_TYPE_STATUS, [
7777
'idea_id' => (int) $idea_id,
7878
'status' => (int) $status,
79+
'user_id' => (int) $this->user->data['user_id'],
7980
]);
8081
}
8182

‎language/en/common.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
'IDEA_DELETED' => 'Idea successfully deleted.',
3939
'IDEA_LIST' => 'Idea List',
4040
'IDEA_NOT_FOUND' => 'Idea not found',
41-
'IDEA_STATUS_CHANGE' => 'Your idea <strong>“%s”</strong> has been changed to:',
41+
'IDEA_STATUS_CHANGE' => '<strong>Idea updated</strong> by %1$s to <em>%2$s</em> for:',
4242
'IDEA_STORED_MOD' => 'Your idea has been submitted successfully, but it will need to be approved by a moderator before it is publicly viewable. You will be notified when your idea has been approved.<br /><br /><a href="%s">Return to Ideas</a>.',
4343
'IDEAS_TITLE' => 'phpBB Ideas',
4444
'IDEAS_NOT_AVAILABLE' => 'Ideas is not available at this time.',

‎language/en/email/status_notification.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Subject: Idea status - "{IDEA_TITLE}"
22

33
Hello {USERNAME},
44

5-
Your Idea "{IDEA_TITLE}" at "{SITENAME}" was changed to {STATUS}.
5+
The status of your Idea topic "{IDEA_TITLE}" at "{SITENAME}" has been updated by {UPDATED_BY} to {STATUS}.
66

77
If you want to view the Idea, click the following link:
88
{U_VIEW_IDEA}

‎notification/type/status.php‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public function find_users_for_notification($type_data, $options = [])
132132
*/
133133
public function users_to_query()
134134
{
135-
return [$this->get_data('idea_author')];
135+
return [$this->get_data('updater_id')];
136136
}
137137

138138
/**
@@ -144,15 +144,22 @@ public function get_title()
144144
{
145145
$this->language->add_lang('common', 'phpbb/ideas');
146146
}
147-
return $this->language->lang($this->language_key, $this->get_data('idea_title'));
147+
148+
$status = $this->language->lang(ext::status_name($this->get_data('status')));
149+
$username = $this->user_loader->get_username($this->get_data('updater_id'), 'no_profile');
150+
151+
return $this->language->lang($this->language_key, $username, $status);
148152
}
149153

150154
/**
151155
* {@inheritDoc}
152156
*/
153157
public function get_reference()
154158
{
155-
return $this->language->lang(ext::status_name($this->get_data('status')));
159+
return $this->language->lang(
160+
'NOTIFICATION_REFERENCE',
161+
censor_text($this->get_data('idea_title'))
162+
);
156163
}
157164

158165
/**
@@ -170,7 +177,7 @@ public function get_url($reference_type = UrlGeneratorInterface::ABSOLUTE_PATH)
170177
*/
171178
public function get_avatar()
172179
{
173-
return $this->user_loader->get_avatar($this->get_data('idea_author'), false, true);
180+
return $this->user_loader->get_avatar($this->get_data('updater_id'), false, true);
174181
}
175182

176183
/**
@@ -189,6 +196,7 @@ public function get_email_template_variables()
189196
return [
190197
'IDEA_TITLE' => html_entity_decode(censor_text($this->get_data('idea_title')), ENT_COMPAT),
191198
'STATUS' => html_entity_decode($this->language->lang(ext::status_name($this->get_data('status'))), ENT_COMPAT),
199+
'UPDATED_BY' => html_entity_decode($this->user_loader->get_username($this->get_data('updater_id'), 'username'), ENT_COMPAT),
192200
'U_VIEW_IDEA' => $this->get_url(UrlGeneratorInterface::ABSOLUTE_URL),
193201
];
194202
}
@@ -202,8 +210,8 @@ public function create_insert_array($type_data, $pre_create_data = [])
202210

203211
$this->set_data('idea_id', (int) $type_data['idea_id']);
204212
$this->set_data('status', (int) $type_data['status']);
213+
$this->set_data('updater_id', (int) $type_data['user_id']);
205214
$this->set_data('idea_title', $idea ? $idea['idea_title'] : '');
206-
$this->set_data('idea_author', $idea ? (int) $idea['idea_author'] : 0);
207215

208216
parent::create_insert_array($type_data, $pre_create_data);
209217
}

‎tests/notification/status_test.php‎

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ public function test_find_users_for_notification_idea_not_found()
167167

168168
public function test_get_avatar_with_author()
169169
{
170-
$this->setNotificationData(['idea_author' => 5]);
170+
$this->setNotificationData(['updater_id' => 5]);
171171

172172
$this->user_loader->expects($this->once())
173173
->method('get_avatar')
@@ -179,36 +179,57 @@ public function test_get_avatar_with_author()
179179

180180
public function test_get_avatar_without_author()
181181
{
182-
$this->setNotificationData(['idea_author' => 0]);
182+
$this->setNotificationData(['updater_id' => 0]);
183183
$this->assertEquals('', $this->notification_type->get_avatar());
184184
}
185185

186186
public function test_users_to_query()
187187
{
188-
$this->setNotificationData(['idea_author' => 0]);
188+
$this->setNotificationData(['updater_id' => 0]);
189189
$this->assertEquals([0], $this->notification_type->users_to_query());
190190
}
191191

192192
public function test_get_title()
193193
{
194-
$this->setNotificationData(['idea_title' => 'Test Idea']);
194+
$this->setNotificationData([
195+
'updater_id' => 123,
196+
'status' => ext::$statuses['IN_PROGRESS']
197+
]);
195198

196199
$this->language->expects($this->once())
197200
->method('is_set')
198201
->with('IDEA_STATUS_CHANGE')
199202
->willReturn(true);
200203

201-
$this->language->expects($this->once())
204+
$this->language->expects($this->exactly(2))
202205
->method('lang')
203-
->with('IDEA_STATUS_CHANGE', 'Test Idea')
204-
->willReturn('Status changed for: Test Idea');
206+
->willReturnCallback(function($key, ...$args) {
207+
if ($key === 'IN_PROGRESS')
208+
{
209+
return 'In Progress';
210+
}
211+
if ($key === 'IDEA_STATUS_CHANGE' && $args[0] === 'TestUser' && $args[1] === 'In Progress')
212+
{
213+
return 'TestUser changed status to In Progress';
214+
}
215+
return '';
216+
});
205217

206-
$this->assertEquals('Status changed for: Test Idea', $this->notification_type->get_title());
218+
$this->user_loader->expects($this->once())
219+
->method('get_username')
220+
->with(123, 'no_profile')
221+
->willReturn('TestUser');
222+
223+
$result = $this->notification_type->get_title();
224+
$this->assertEquals('TestUser changed status to In Progress', $result);
207225
}
208226

209227
public function test_get_title_loads_language()
210228
{
211-
$this->setNotificationData(['idea_title' => 'Test Idea']);
229+
$this->setNotificationData([
230+
'updater_id' => 456,
231+
'status' => ext::$statuses['IMPLEMENTED']
232+
]);
212233

213234
$this->language->expects($this->once())
214235
->method('is_set')
@@ -219,24 +240,39 @@ public function test_get_title_loads_language()
219240
->method('add_lang')
220241
->with('common', 'phpbb/ideas');
221242

222-
$this->language->expects($this->once())
243+
$this->language->expects($this->exactly(2))
223244
->method('lang')
224-
->with('IDEA_STATUS_CHANGE', 'Test Idea')
225-
->willReturn('Status changed for: Test Idea');
245+
->willReturnCallback(function($key, ...$args) {
246+
if ($key === 'IMPLEMENTED')
247+
{
248+
return 'Implemented';
249+
}
250+
if ($key === 'IDEA_STATUS_CHANGE' && $args[0] === 'AdminUser' && $args[1] === 'Implemented')
251+
{
252+
return 'AdminUser changed status to Implemented';
253+
}
254+
return '';
255+
});
256+
257+
$this->user_loader->expects($this->once())
258+
->method('get_username')
259+
->with(456, 'no_profile')
260+
->willReturn('AdminUser');
226261

227-
$this->assertEquals('Status changed for: Test Idea', $this->notification_type->get_title());
262+
$result = $this->notification_type->get_title();
263+
$this->assertEquals('AdminUser changed status to Implemented', $result);
228264
}
229265

230266
public function test_get_reference()
231267
{
232-
$this->setNotificationData(['status' => 2]);
268+
$this->setNotificationData(['idea_title' => 'Test Idea']);
233269

234270
$this->language->expects($this->once())
235271
->method('lang')
236-
->with('IN_PROGRESS')
237-
->willReturn('In Progress');
272+
->with('NOTIFICATION_REFERENCE', 'Test Idea')
273+
->willReturn('“Test Idea”');
238274

239-
$this->assertEquals('In Progress', $this->notification_type->get_reference());
275+
$this->assertEquals('“Test Idea”', $this->notification_type->get_reference());
240276
}
241277

242278
public function test_get_url()
@@ -261,7 +297,8 @@ public function test_get_email_template_variables()
261297
$this->setNotificationData([
262298
'idea_title' => 'Test & Idea',
263299
'status' => 3,
264-
'idea_id' => 10
300+
'idea_id' => 10,
301+
'updater_id' => 123,
265302
]);
266303

267304
$this->helper->expects($this->once())
@@ -274,11 +311,17 @@ public function test_get_email_template_variables()
274311
->with('IMPLEMENTED')
275312
->willReturn('Implemented');
276313

314+
$this->user_loader->expects($this->once())
315+
->method('get_username')
316+
->with(123, 'username')
317+
->willReturn('TestUser');
318+
277319
$result = $this->notification_type->get_email_template_variables();
278320

279321
$expected = [
280322
'IDEA_TITLE' => 'Test & Idea',
281323
'STATUS' => 'Implemented',
324+
'UPDATED_BY' => 'TestUser',
282325
'U_VIEW_IDEA' => '/ideas/10',
283326
];
284327

@@ -289,32 +332,28 @@ public function test_create_insert_array()
289332
{
290333
$type_data = [
291334
'idea_id' => 7,
292-
'status' => 4
335+
'status' => 4,
336+
'user_id' => 3
293337
];
294338
$idea_data = [
295-
'idea_title' => 'Sample Idea',
296-
'idea_author' => 3
339+
'idea_title' => 'Sample Idea'
297340
];
298341

299342
$this->idea_factory->expects($this->once())
300343
->method('get_idea')
301344
->with(7)
302345
->willReturn($idea_data);
303346

304-
// Use reflection to access set_data calls
305-
$reflection = new \ReflectionClass($this->notification_type);
306-
$set_data_method = $reflection->getMethod('set_data');
307-
$set_data_method->setAccessible(true);
308-
309347
$this->notification_type->create_insert_array($type_data);
310348

311349
// Verify data was set by checking get_data
350+
$reflection = new \ReflectionClass($this->notification_type);
312351
$get_data_method = $reflection->getMethod('get_data');
313352
$get_data_method->setAccessible(true);
314353

315354
$this->assertEquals(7, $get_data_method->invoke($this->notification_type, 'idea_id'));
316355
$this->assertEquals(4, $get_data_method->invoke($this->notification_type, 'status'));
356+
$this->assertEquals(3, $get_data_method->invoke($this->notification_type, 'updater_id'));
317357
$this->assertEquals('Sample Idea', $get_data_method->invoke($this->notification_type, 'idea_title'));
318-
$this->assertEquals(3, $get_data_method->invoke($this->notification_type, 'idea_author'));
319358
}
320359
}

0 commit comments

Comments
 (0)