Skip to content

Commit c088d1a

Browse files
iMattProiMattPro
committed
Refactor to use idea id so we can update/delete notifications
Signed-off-by: Matt Friedman <maf675@gmail.com>
1 parent 936bf7c commit c088d1a

6 files changed

Lines changed: 55 additions & 103 deletions

File tree

‎factory/idea.php‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,8 @@ public function set_status($idea_id, $status)
7272
$this->update_idea_data($sql_ary, $idea_id, $this->table_ideas);
7373

7474
// Send a notification
75-
// Increment our notifications sent counter
76-
$this->config->increment('ideas_status_notifications_id', 1);
77-
$this->notification_manager->add_notifications('phpbb.ideas.notification.type.status', [
78-
'item_id' => (int) $this->config['ideas_status_notifications_id'],
75+
$method = $this->notification_exists($idea_id) ? 'update_notifications' : 'add_notifications';
76+
$this->notification_manager->$method('phpbb.ideas.notification.type.status', [
7977
'idea_id' => (int) $idea_id,
8078
'status' => (int) $status,
8179
]);
@@ -271,8 +269,14 @@ public function delete($id, $topic_id = 0)
271269
// Delete idea
272270
$deleted = $this->delete_idea_data($id, $this->table_ideas);
273271

274-
// Delete votes
275-
$this->delete_idea_data($id, $this->table_votes);
272+
if ($deleted)
273+
{
274+
// Delete votes
275+
$this->delete_idea_data($id, $this->table_votes);
276+
277+
// Delete notifications
278+
$this->notification_manager->delete_notifications('phpbb.ideas.notification.type.status', $id);
279+
}
276280

277281
return $deleted;
278282
}
@@ -449,4 +453,24 @@ protected function get_users_vote($idea_id, $user_id)
449453

450454
return $row;
451455
}
456+
457+
/**
458+
* Check if a notification already exists
459+
*
460+
* @param int $item_id The item identifier for the notification
461+
* @return bool
462+
*/
463+
protected function notification_exists($item_id)
464+
{
465+
$sql = 'SELECT notification_id
466+
FROM ' . NOTIFICATIONS_TABLE . '
467+
WHERE item_id = ' . (int) $item_id . '
468+
AND notification_type_id = ' . $this->notification_manager->get_notification_type_id('phpbb.ideas.notification.type.status');
469+
470+
$result = $this->db->sql_query_limit($sql, 1);
471+
$row = $this->db->sql_fetchrow($result);
472+
$this->db->sql_freeresult($result);
473+
474+
return $row !== false;
475+
}
452476
}

‎migrations/m14_notification_data.php‎

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎notification/type/status.php‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public function get_type()
8989
*/
9090
public static function get_item_id($type_data)
9191
{
92-
return (int) $type_data['item_id'];
92+
return (int) $type_data['idea_id'];
9393
}
9494

9595
/**
@@ -193,29 +193,14 @@ public function get_email_template_variables()
193193
/**
194194
* {@inheritDoc}
195195
*/
196-
public function pre_create_insert_array($type_data, $notify_users)
196+
public function create_insert_array($type_data, $pre_create_data = [])
197197
{
198-
$pre_create_data = [];
199-
200198
$idea = $this->idea->get_idea($type_data['idea_id']);
201-
if ($idea !== false)
202-
{
203-
$pre_create_data['idea_title'] = $idea['idea_title'];
204-
$pre_create_data['idea_author'] = $idea['idea_author'];
205-
}
206-
207-
return $pre_create_data;
208-
}
209199

210-
/**
211-
* {@inheritDoc}
212-
*/
213-
public function create_insert_array($type_data, $pre_create_data = [])
214-
{
215200
$this->set_data('idea_id', $type_data['idea_id']);
216201
$this->set_data('status', $type_data['status']);
217-
$this->set_data('idea_title', $pre_create_data['idea_title']);
218-
$this->set_data('idea_author', $pre_create_data['idea_author']);
202+
$this->set_data('idea_title', $idea['idea_title']);
203+
$this->set_data('idea_author', $idea['idea_author']);
219204

220205
parent::create_insert_array($type_data, $pre_create_data);
221206
}

‎tests/ideas/delete_idea_test.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ public function delete_test_data()
3232
*/
3333
public function test_delete($idea_id)
3434
{
35+
$this->notification_manager->expects($this->once())
36+
->method('delete_notifications');
37+
3538
$object = $this->get_idea_object();
3639

3740
// delete idea
@@ -70,6 +73,9 @@ public function delete_fails_test_data()
7073
*/
7174
public function test_delete_fails($idea_id)
7275
{
76+
$this->notification_manager->expects($this->never())
77+
->method('delete_notifications');
78+
7379
$object = $this->get_idea_object();
7480

7581
self::assertFalse($object->delete($idea_id));

‎tests/ideas/idea_attributes_test.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ public function set_status_test_data()
6161
*/
6262
public function test_set_status($idea_id, $status)
6363
{
64+
$this->notification_manager->expects($this->once())
65+
->method('get_notification_type_id')
66+
->with('phpbb.ideas.notification.type.status')
67+
->willReturn(1);
68+
6469
$object = $this->get_idea_object();
6570

6671
$object->set_status($idea_id, $status);

‎tests/notification/status_test.php‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public function test_is_available_without_permission()
114114

115115
public function test_get_item_id()
116116
{
117-
$type_data = ['item_id' => 123];
117+
$type_data = ['idea_id' => 123];
118118
$this->assertEquals(123, status::get_item_id($type_data));
119119
}
120120

@@ -284,70 +284,36 @@ public function test_get_email_template_variables()
284284
$this->assertEquals($expected, $result);
285285
}
286286

287-
public function test_pre_create_insert_array()
287+
public function test_create_insert_array()
288288
{
289-
$type_data = ['idea_id' => 5];
290-
$notify_users = [];
289+
$type_data = [
290+
'idea_id' => 7,
291+
'status' => 4
292+
];
291293
$idea_data = [
292294
'idea_title' => 'Sample Idea',
293295
'idea_author' => 3
294296
];
295297

296298
$this->idea_factory->expects($this->once())
297299
->method('get_idea')
298-
->with(5)
300+
->with(7)
299301
->willReturn($idea_data);
300302

301-
$result = $this->notification_type->pre_create_insert_array($type_data, $notify_users);
302-
303-
$expected = [
304-
'idea_title' => 'Sample Idea',
305-
'idea_author' => 3
306-
];
307-
308-
$this->assertEquals($expected, $result);
309-
}
310-
311-
public function test_pre_create_insert_array_idea_not_found()
312-
{
313-
$type_data = ['idea_id' => 999];
314-
$notify_users = [];
315-
316-
$this->idea_factory->expects($this->once())
317-
->method('get_idea')
318-
->with(999)
319-
->willReturn(false);
320-
321-
$result = $this->notification_type->pre_create_insert_array($type_data, $notify_users);
322-
$this->assertEquals([], $result);
323-
}
324-
325-
public function test_create_insert_array()
326-
{
327-
$type_data = [
328-
'item_id' => 1,
329-
'idea_id' => 7,
330-
'status' => 4
331-
];
332-
$pre_create_data = [
333-
'idea_title' => 'Another Idea',
334-
'idea_author' => 8
335-
];
336-
337303
// Use reflection to access set_data calls
338304
$reflection = new \ReflectionClass($this->notification_type);
339305
$set_data_method = $reflection->getMethod('set_data');
340306
$set_data_method->setAccessible(true);
341307

342-
$this->notification_type->create_insert_array($type_data, $pre_create_data);
308+
$this->notification_type->create_insert_array($type_data);
343309

344310
// Verify data was set by checking get_data
345311
$get_data_method = $reflection->getMethod('get_data');
346312
$get_data_method->setAccessible(true);
347313

348314
$this->assertEquals(7, $get_data_method->invoke($this->notification_type, 'idea_id'));
349315
$this->assertEquals(4, $get_data_method->invoke($this->notification_type, 'status'));
350-
$this->assertEquals('Another Idea', $get_data_method->invoke($this->notification_type, 'idea_title'));
351-
$this->assertEquals(8, $get_data_method->invoke($this->notification_type, 'idea_author'));
316+
$this->assertEquals('Sample Idea', $get_data_method->invoke($this->notification_type, 'idea_title'));
317+
$this->assertEquals(3, $get_data_method->invoke($this->notification_type, 'idea_author'));
352318
}
353319
}

0 commit comments

Comments
 (0)