News: Bug fixes, cleanup, comments & formatting

Use more magically available methods and properties
Fixed atom feed and stats not using the new model

db/migrations/2019_10_15_000000_create_news_table.php

@@ -1,10 +1,12 @@
 <?php
+
 declare(strict_types=1);
 
 namespace Engelsystem\Migrations;
 
 use Carbon\Carbon;
 use Engelsystem\Database\Migration\Migration;
+use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Schema\Blueprint;
 use stdClass;
 
@@ -13,7 +15,8 @@ use stdClass;
  */
 class CreateNewsTable extends Migration
 {
-    use ChangesReferences, Reference;
+    use ChangesReferences;
+    use Reference;
 
     /**
      * Creates the news table, copies the data and drops the News table.
@@ -23,7 +26,7 @@ class CreateNewsTable extends Migration
         $hasPreviousNewsTable = $this->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,

includes/pages/admin_news.php

@@ -1,13 +1,12 @@
 <?php
 
-use Engelsystem\Models\News\News;
+use Engelsystem\Models\News;
 
 /**
  * @return string
  */
 function admin_news()
 {
-    $user = auth()->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])

includes/pages/user_atom.php

@@ -1,7 +1,8 @@
 <?php
 
-use Engelsystem\Database\DB;
 use Engelsystem\Http\Exceptions\HttpForbidden;
+use Engelsystem\Models\News;
+use Illuminate\Database\Eloquent\Collection;
 
 /**
  * Publically available page to feed the news to feed readers
@@ -23,15 +24,11 @@ function user_atom()
         throw new HttpForbidden('Not allowed', ['content-type' => '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 = '<?xml version="1.0" encoding="utf-8"?>
   <feed xmlns="http://www.w3.org/2005/Atom">
   <title>' . config('app_name') . '</title>
@@ -55,7 +54,7 @@ function make_atom_entries_from_news($news_entries)
             $request->getRequestUri()
         ))
         . '</id>
-  <updated>' . date('Y-m-d\TH:i:sP', $news_entries[0]['Datum']) . '</updated>' . "\n";
+  <updated>' . $updatedAt . '</updated>' . "\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 '
   <entry>
-    <title>' . htmlspecialchars($news_entry['Betreff']) . '</title>
-    <link href="' . page_link_to('news_comments', ['nid' => $news_entry['ID']]) . '"/>
+    <title>' . htmlspecialchars($news->title) . '</title>
+    <link href="' . page_link_to('news_comments', ['nid' => $news->id]) . '"/>
     <id>' . preg_replace(
             '#^https?://#',
             '',
-            page_link_to('news_comments', ['nid' => $news_entry['ID']])
+            page_link_to('news_comments', ['nid' => $news->id])
         ) . '</id>
-    <updated>' . date('Y-m-d\TH:i:sP', $news_entry['Datum']) . '</updated>
-    <summary type="html">' . htmlspecialchars($news_entry['Text']) . '</summary>
+    <updated>' . $news->updated_at->format('Y-m-d\TH:i:sP') . '</updated>
+    <summary type="html">' . htmlspecialchars($news->text) . '</summary>
   </entry>' . "\n";
 }

includes/pages/user_news.php

@@ -1,7 +1,7 @@
 <?php
 
 use Engelsystem\Database\DB;
-use Engelsystem\Models\News\News;
+use Engelsystem\Models\News;
 use Engelsystem\Models\User\User;
 
 /**
@@ -43,7 +43,7 @@ function user_meetings()
         $page = 0;
     }
 
-    $news = News::where('is_meeting', true)
+    $news = News::whereIsMeeting(true)
         ->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 .= '<div class="text-center">' . '<ul class="pagination">';
     for ($i = 0; $i < $dis_rows; $i++) {
         if ($request->has('page') && $i == $request->input('page', 0)) {
@@ -110,7 +110,7 @@ function display_news(News $news): string
     }
     $html .= '<span class="glyphicon glyphicon-time"></span> ' . $news->created_at->format('Y-m-d H:i') . '&emsp;';
 
-    $html .= User_Nick_render(User::find($news->user_id));
+    $html .= User_Nick_render($news->user);
     if ($page != 'news_comments') {
         $html .= '&emsp;<a href="' . page_link_to('news_comments', ['nid' => $news->id]) . '">'
             . '<span class="glyphicon glyphicon-comment"></span> '
@@ -137,9 +137,8 @@ function user_news_comments()
     if (
         $request->has('nid')
         && preg_match('/^\d{1,}$/', $nid)
-        && News::where('id', $request->input('nid'))->count() > 0
+        && $news = News::find($nid)
     ) {
-        $news = News::find('id');
         if ($request->hasPostData('submit') && $request->has('text')) {
             $text = $request->input('text');
             DB::insert('
@@ -180,7 +179,7 @@ function user_news_comments()
         $html .= form([
             form_textarea('text', __('Message'), ''),
             form_submit('submit', __('Save'))
-        ], page_link_to('news_comments', ['nid' => $news['ID']]));
+        ], page_link_to('news_comments', ['nid' => $news->id]));
     } else {
         $html .= __('Invalid request.');
     }
@@ -199,25 +198,21 @@ function user_news()
 
     $html = '<div class="col-md-12"><h1>' . news_title() . '</h1>' . msg();
 
-    $isMeeting = $request->postData('treffen');
+    $isMeeting = $request->postData('treffen', false);
     if ($request->has('text') && $request->has('betreff') && auth()->can('admin_news')) {
-        if (!$request->has('treffen')) {
-            $isMeeting = 0;
-        }
-
         $text = $request->postData('text');
         if (!auth()->can('admin_news_html')) {
             $text = strip_tags($text);
         }
 
-        News::create([
+        $news = News::create([
             'title'      => strip_tags($request->postData('betreff')),
             'text'       => $text,
             'user_id'    => $user->id,
-            'is_meeting' => !!$isMeeting,
+            'is_meeting' => (bool)$isMeeting,
         ]);
 
-        engelsystem_log('Created news: ' . $request->postData('betreff') . ', treffen: ' . $isMeeting);
+        engelsystem_log('Created news: ' . $news->title . ', is meeting: ' . ($news->is_meeting ? 'yes' : 'no'));
         success(__('Entry saved.'));
         redirect(page_link_to('news'));
     }

src/Controllers/Metrics/Stats.php

@@ -5,6 +5,7 @@ namespace Engelsystem\Controllers\Metrics;
 use Carbon\Carbon;
 use Engelsystem\Database\Database;
 use Engelsystem\Models\EventConfig;
+use Engelsystem\Models\News;
 use Illuminate\Database\Query\Builder as QueryBuilder;
 use Illuminate\Database\Query\Expression as QueryExpression;
 
@@ -209,16 +210,10 @@ class Stats
     /**
      * @param bool $meeting
      * @return int
-     * @codeCoverageIgnore
      */
     public function announcements($meeting = null)
     {
-        $query = $this
-            ->getQuery('News');
-
-        if (!is_null($meeting)) {
-            $query->where('Treffen', '=', $meeting);
-        }
+        $query = is_null($meeting) ? News::query() : News::whereIsMeeting($meeting);
 
         return $query->count();
     }

src/Models/News.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Engelsystem\Models;
+
+use Carbon\Carbon;
+use Engelsystem\Models\User\UsesUserModel;
+use Illuminate\Database\Query\Builder as QueryBuilder;
+
+/**
+ * @property int         $id
+ * @property string      $title
+ * @property string      $text
+ * @property bool        $is_meeting
+ * @property Carbon|null $created_at
+ * @property Carbon|null $updated_at
+ *
+ * @method static QueryBuilder|LogEntry[] whereId($value)
+ * @method static QueryBuilder|LogEntry[] whereTitle($value)
+ * @method static QueryBuilder|LogEntry[] whereText($value)
+ * @method static QueryBuilder|LogEntry[] whereIsMeeting($value)
+ * @method static QueryBuilder|LogEntry[] whereCreatedAt($value)
+ * @method static QueryBuilder|LogEntry[] whereUpdatedAt($value)
+ */
+class News extends BaseModel
+{
+    use UsesUserModel;
+
+    /** @var bool Enable timestamps */
+    public $timestamps = true;
+
+    /** @var array */
+    protected $casts = [
+        'is_meeting' => 'boolean',
+    ];
+
+    /** @var array */
+    protected $fillable = [
+        'title',
+        'text',
+        'is_meeting',
+        'user_id',
+    ];
+}

src/Models/News/News.php

@@ -1,30 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace Engelsystem\Models\News;
-
-use Engelsystem\Models\User\UsesUserModel;
-use Illuminate\Database\Eloquent\Model;
-
-/**
- * This class represents a news item.
- */
-class News extends Model
-{
-    use UsesUserModel;
-
-    protected $casts = [
-        'is_meeting' => 'boolean',
-    ];
-
-    protected $attributes = [
-        'is_meeting' => false,
-    ];
-
-    protected $fillable = [
-        'title',
-        'text',
-        'is_meeting',
-        'user_id',
-    ];
-}

src/Models/User/User.php

@@ -4,7 +4,7 @@ namespace Engelsystem\Models\User;
 
 use Carbon\Carbon;
 use Engelsystem\Models\BaseModel;
-use Engelsystem\Models\News\News;
+use Engelsystem\Models\News;
 use Illuminate\Database\Eloquent\Relations\HasMany;
 use Illuminate\Database\Eloquent\Relations\HasOne;
 use Illuminate\Database\Query\Builder as QueryBuilder;

tests/Unit/Controllers/Metrics/StatsTest.php

@@ -5,6 +5,7 @@ namespace Engelsystem\Test\Unit\Controllers\Metrics;
 use Carbon\Carbon;
 use Engelsystem\Controllers\Metrics\Stats;
 use Engelsystem\Models\LogEntry;
+use Engelsystem\Models\News;
 use Engelsystem\Models\User\PasswordReset;
 use Engelsystem\Models\User\PersonalData;
 use Engelsystem\Models\User\State;
@@ -26,7 +27,6 @@ class StatsTest extends TestCase
      */
     public function testNewUsers()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         $stats = new Stats($this->database);
@@ -38,7 +38,6 @@ class StatsTest extends TestCase
      */
     public function testVouchers()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         $stats = new Stats($this->database);
@@ -50,7 +49,6 @@ class StatsTest extends TestCase
      */
     public function testTshirts()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         $stats = new Stats($this->database);
@@ -63,7 +61,6 @@ class StatsTest extends TestCase
      */
     public function testTshirtSizes()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         $stats = new Stats($this->database);
@@ -75,12 +72,30 @@ class StatsTest extends TestCase
         ]), $sizes);
     }
 
+
+    /**
+     * @covers \Engelsystem\Controllers\Metrics\Stats::announcements
+     */
+    public function testAnnouncements()
+    {
+        $this->addUsers();
+        $newsData = ['title' => 'Test', 'text' => 'Foo Bar', 'user_id' => 1];
+
+        (new News($newsData))->save();
+        (new News($newsData))->save();
+        (new News($newsData + ['is_meeting' => true]))->save();
+
+        $stats = new Stats($this->database);
+        $this->assertEquals(3, $stats->announcements());
+        $this->assertEquals(2, $stats->announcements(false));
+        $this->assertEquals(1, $stats->announcements(true));
+    }
+
     /**
      * @covers \Engelsystem\Controllers\Metrics\Stats::arrivedUsers
      */
     public function testArrivedUsers()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         $stats = new Stats($this->database);
@@ -92,8 +107,6 @@ class StatsTest extends TestCase
      */
     public function testSessions()
     {
-        $this->initDatabase();
-
         $this->database
             ->getConnection()
             ->table('sessions')
@@ -114,8 +127,6 @@ class StatsTest extends TestCase
      */
     public function testDatabase()
     {
-        $this->initDatabase();
-
         $stats = new Stats($this->database);
 
         $read = $stats->databaseRead();
@@ -132,8 +143,6 @@ class StatsTest extends TestCase
      */
     public function testLogEntries()
     {
-        $this->initDatabase();
-
         (new LogEntry(['level' => LogLevel::INFO, 'message' => 'Some info']))->save();
         (new LogEntry(['level' => LogLevel::INFO, 'message' => 'Another info']))->save();
         (new LogEntry(['level' => LogLevel::CRITICAL, 'message' => 'A critical error!']))->save();
@@ -152,7 +161,6 @@ class StatsTest extends TestCase
      */
     public function testPasswordResets()
     {
-        $this->initDatabase();
         $this->addUsers();
 
         (new PasswordReset(['user_id' => 1, 'token' => 'loremIpsum123']))->save();
@@ -203,4 +211,14 @@ class StatsTest extends TestCase
             ->associate($user)
             ->save();
     }
+
+    /**
+     * Set up the environment
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->initDatabase();
+    }
 }

tests/Unit/Models/NewsTest.php

@@ -1,7 +1,10 @@
 <?php
+
 declare(strict_types=1);
 
-use Engelsystem\Models\News\News;
+namespace Engelsystem\Test\Unit\Models;
+
+use Engelsystem\Models\News;
 use Engelsystem\Models\User\User;
 use Engelsystem\Test\Unit\HasDatabase;
 use Engelsystem\Test\Unit\TestCase;
@@ -13,14 +16,10 @@ class NewsTest extends TestCase
 {
     use HasDatabase;
 
-    /**
-     * @var array
-     */
+    /** @var array */
     private $newsData;
 
-    /**
-     * @var User
-     */
+    /** @var User */
     private $user;
 
     /**
@@ -31,13 +30,12 @@ class NewsTest extends TestCase
         parent::setUp();
         $this->initDatabase();
 
-        $this->user = User::make([
+        $this->user = (new User())->create([
             'name'     => 'lorem',
             'password' => '',
             'email'    => 'foo@bar.batz',
             'api_key'  => '',
         ]);
-        $this->user->save();
 
         $this->newsData = [
             'title'   => 'test title',
@@ -53,7 +51,8 @@ class NewsTest extends TestCase
      */
     public function testCreateDefault(): void
     {
-        $news = News::create($this->newsData);
+        $news = (new News())->create($this->newsData);
+        $news = $news->find($news->id);
 
         $this->assertSame(1, $news->id);
         $this->assertSame($this->newsData['title'], $news->title);
@@ -68,9 +67,10 @@ class NewsTest extends TestCase
      */
     public function testCreate(): void
     {
-        $news = News::create(
-            $this->newsData + ['is_meeting' => true,]
+        $news = (new News())->create(
+            $this->newsData + ['is_meeting' => true]
         );
+        $news = $news->find($news->id);
 
         $this->assertSame(1, $news->id);
         $this->assertSame($this->newsData['title'], $news->title);

tests/Unit/Models/User/UserTest.php

@@ -3,7 +3,8 @@
 namespace Engelsystem\Test\Unit\Models;
 
 use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts;
-use Engelsystem\Models\News\News;
+use Engelsystem\Models\BaseModel;
+use Engelsystem\Models\News;
 use Engelsystem\Models\User\Contact;
 use Engelsystem\Models\User\HasUserModel;
 use Engelsystem\Models\User\PersonalData;
@@ -19,6 +20,7 @@ class UserTest extends TestCase
     use ArraySubsetAsserts;
     use HasDatabase;
 
+    /** @var string[] */
     protected $data = [
         'name'     => 'lorem',
         'password' => '',
@@ -26,15 +28,6 @@ class UserTest extends TestCase
         'api_key'  => '',
     ];
 
-    /**
-     * Prepare test
-     */
-    protected function setUp(): void
-    {
-        parent::setUp();
-        $this->initDatabase();
-    }
-
     /**
      * @return array
      */
@@ -103,23 +96,25 @@ class UserTest extends TestCase
     }
 
     /**
-     * @covers User::news()
+     * @covers       \Engelsystem\Models\User\User::news()
      *
      * @dataProvider hasManyRelationsProvider
      *
-     * @param string $class Class name of the related models
-     * @param string $name Name of the accessor for the related models
-     * @param array  $data List of the related models
+     * @param string $class     Class name of the related models
+     * @param string $name      Name of the accessor for the related models
+     * @param array  $modelData List of the related models
      */
-    public function testHasManyRelations(string $class, string $name, array $data): void
+    public function testHasManyRelations(string $class, string $name, array $modelData): void
     {
         $user = new User($this->data);
         $user->save();
 
         $relatedModelIds = [];
 
-        foreach ($data as $d) {
-            $stored = $class::create($d + ['user_id' => $user->id]);
+        foreach ($modelData as $data) {
+            /** @var BaseModel $model */
+            $model = (new $class);
+            $stored = $model->create($data + ['user_id' => $user->id]);
             $relatedModelIds[] = $stored->id;
         }
 
@@ -127,7 +122,7 @@ class UserTest extends TestCase
     }
 
     /**
-     * @return array
+     * @return array[]
      */
     public function hasManyRelationsProvider(): array
     {
@@ -150,4 +145,13 @@ class UserTest extends TestCase
             ]
         ];
     }
+
+    /**
+     * Prepare test
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->initDatabase();
+    }
 }