From b878740f80ce7cfe2a0bc53956e3f7e4e0aa2f78 Mon Sep 17 00:00:00 2001 From: Igor Scheller Date: Sun, 10 Nov 2019 21:30:26 +0100 Subject: News: Bug fixes, cleanup, comments & formatting Use more magically available methods and properties Fixed atom feed and stats not using the new model --- .../2019_10_15_000000_create_news_table.php | 43 ++++++++---- includes/pages/admin_news.php | 5 +- includes/pages/user_atom.php | 37 +++++----- includes/pages/user_news.php | 25 +++---- src/Controllers/Metrics/Stats.php | 9 +-- src/Models/News.php | 45 ++++++++++++ src/Models/News/News.php | 30 -------- src/Models/User/User.php | 2 +- tests/Unit/Controllers/Metrics/StatsTest.php | 42 ++++++++---- tests/Unit/Models/News/NewsTest.php | 80 ---------------------- tests/Unit/Models/NewsTest.php | 80 ++++++++++++++++++++++ tests/Unit/Models/User/UserTest.php | 40 ++++++----- 12 files changed, 238 insertions(+), 200 deletions(-) create mode 100644 src/Models/News.php delete mode 100644 src/Models/News/News.php delete mode 100644 tests/Unit/Models/News/NewsTest.php create mode 100644 tests/Unit/Models/NewsTest.php diff --git a/db/migrations/2019_10_15_000000_create_news_table.php b/db/migrations/2019_10_15_000000_create_news_table.php index d6b93265..c87972ef 100644 --- a/db/migrations/2019_10_15_000000_create_news_table.php +++ b/db/migrations/2019_10_15_000000_create_news_table.php @@ -1,10 +1,12 @@ schema->hasTable('News'); if ($hasPreviousNewsTable) { - // rename because some SQL DBMS handle identifiers case insensitive + // Rename because some SQL DBMS handle identifiers case insensitive $this->schema->rename('News', 'PreviousNews'); } @@ -47,7 +50,7 @@ class CreateNewsTable extends Migration */ public function down(): void { - // rename because some SQL DBMS handle identifiers case insensitive + // Rename as some SQL DBMS handle identifiers case insensitive $this->schema->rename('news', 'new_news'); $this->createPreviousNewsTable(); @@ -59,9 +62,13 @@ class CreateNewsTable extends Migration 'ID', 'unsignedInteger' ); + $this->schema->drop('new_news'); } + /** + * @return void + */ private function createNewNewsTable(): void { $this->schema->create('news', function (Blueprint $table) { @@ -74,17 +81,20 @@ class CreateNewsTable extends Migration }); } + /** + * @return void + */ private function copyPreviousToNewNewsTable(): void { + $connection = $this->schema->getConnection(); /** @var stdClass[] $previousNewsRecords */ - $previousNewsRecords = $this->schema - ->getConnection() + $previousNewsRecords = $connection ->table('PreviousNews') ->get(); foreach ($previousNewsRecords as $previousNews) { $date = Carbon::createFromTimestamp($previousNews->Datum); - $this->schema->getConnection()->table('news')->insert([ + $connection->table('news')->insert([ 'id' => $previousNews->ID, 'title' => $previousNews->Betreff, 'text' => $previousNews->Text, @@ -96,6 +106,9 @@ class CreateNewsTable extends Migration } } + /** + * @return void + */ private function createPreviousNewsTable(): void { $this->schema->create('News', function (Blueprint $table) { @@ -104,19 +117,19 @@ class CreateNewsTable extends Migration $table->string('Betreff', 150) ->default(''); $table->text('Text'); - $table->boolean('Treffen'); - $table->unsignedInteger('UID'); - $table->foreign('UID') - ->references('id') - ->on('users'); + $this->references($table, 'users', 'UID'); + $table->boolean('Treffen')->default(false); }); } + /** + * @return void + */ private function copyNewToPreviousNewsTable(): void { - /** @var stdClass[] $newsRecords */ - $newsRecords = $this->schema - ->getConnection() + $connection = $this->schema->getConnection(); + /** @var Collection[]|stdClass[] $newsRecords */ + $newsRecords = $connection ->table('new_news') ->get(); @@ -124,7 +137,7 @@ class CreateNewsTable extends Migration $date = Carbon::createFromFormat('Y-m-d H:i:s', $newsRecord->created_at) ->getTimestamp(); - $this->schema->getConnection()->table('News')->insert([ + $connection->table('News')->insert([ 'ID' => $newsRecord->id, 'Datum' => $date, 'Betreff' => $newsRecord->title, diff --git a/includes/pages/admin_news.php b/includes/pages/admin_news.php index 75d8291e..1d49af80 100644 --- a/includes/pages/admin_news.php +++ b/includes/pages/admin_news.php @@ -1,13 +1,12 @@ user(); $request = request(); if (!$request->has('action')) { @@ -45,7 +44,7 @@ function admin_news() form_info(__('Author'), User_Nick_render($user_source)), form_text('eBetreff', __('Subject'), $news->title), form_textarea('eText', __('Message'), $news->text), - form_checkbox('eTreffen', __('Meeting'), $news->is_meeting === true, 1), + form_checkbox('eTreffen', __('Meeting'), $news->is_meeting, 1), form_submit('submit', __('Save')) ], page_link_to('admin_news', ['action' => 'save', 'id' => $news_id]) diff --git a/includes/pages/user_atom.php b/includes/pages/user_atom.php index a491fea7..9a4d65a5 100644 --- a/includes/pages/user_atom.php +++ b/includes/pages/user_atom.php @@ -1,7 +1,8 @@ 'text/text']); } - $news = DB::select(' - SELECT * - FROM `News` - ' . (!$request->has('meetings') ? '' : 'WHERE `Treffen` = 1 ') . ' - ORDER BY `ID` - DESC LIMIT ' . (int)config('display_news') - ); - - $output = make_atom_entries_from_news($news); + $news = $request->has('meetings') ? News::whereIsMeeting((bool)$request->get('meetings', false)) : News::query(); + $news + ->limit((int)config('display_news')) + ->orderByDesc('updated_at'); + $output = make_atom_entries_from_news($news->get()); header('Content-Type: application/atom+xml; charset=utf-8'); header('Content-Length: ' . strlen($output)); @@ -39,12 +36,14 @@ function user_atom() } /** - * @param array[] $news_entries + * @param News[]|Collection $news_entries * @return string */ function make_atom_entries_from_news($news_entries) { $request = app('request'); + $updatedAt = isset($news_entries[0]) ? $news_entries[0]->updated_at->format('Y-m-d\TH:i:sP') : '0000:00:00T00:00:00+00:00'; + $html = ' ' . config('app_name') . ' @@ -55,7 +54,7 @@ function make_atom_entries_from_news($news_entries) $request->getRequestUri() )) . ' - ' . date('Y-m-d\TH:i:sP', $news_entries[0]['Datum']) . '' . "\n"; + ' . $updatedAt . '' . "\n"; foreach ($news_entries as $news_entry) { $html .= make_atom_entry_from_news($news_entry); } @@ -64,21 +63,21 @@ function make_atom_entries_from_news($news_entries) } /** - * @param array $news_entry + * @param News $news * @return string */ -function make_atom_entry_from_news($news_entry) +function make_atom_entry_from_news(News $news) { return ' - ' . htmlspecialchars($news_entry['Betreff']) . ' - + ' . htmlspecialchars($news->title) . ' + ' . preg_replace( '#^https?://#', '', - page_link_to('news_comments', ['nid' => $news_entry['ID']]) + page_link_to('news_comments', ['nid' => $news->id]) ) . ' - ' . date('Y-m-d\TH:i:sP', $news_entry['Datum']) . ' - ' . htmlspecialchars($news_entry['Text']) . ' + ' . $news->updated_at->format('Y-m-d\TH:i:sP') . ' + ' . htmlspecialchars($news->text) . ' ' . "\n"; } diff --git a/includes/pages/user_news.php b/includes/pages/user_news.php index f67896da..643d9d04 100644 --- a/includes/pages/user_news.php +++ b/includes/pages/user_news.php @@ -1,7 +1,7 @@ orderBy('created_at', 'DESC') ->limit($display_news) ->offset($page * $display_news) @@ -53,7 +53,7 @@ function user_meetings() $html .= display_news($entry); } - $dis_rows = ceil(News::where('is_meeting', true)->count() / $display_news); + $dis_rows = ceil(News::whereIsMeeting(true)->count() / $display_news); $html .= '
' . '