From 95fb1e8ed6b5cc2332e17408184779a60dc66380 Mon Sep 17 00:00:00 2001 From: Lewis Larsen Date: Tue, 30 Jul 2024 21:19:09 +0100 Subject: [PATCH] fix: Improved how we check for banner dismissals --- app/Console/Commands/FetchNewFeatures.php | 138 +++++++++++++----- app/Livewire/Other/NewFeatureBanner.php | 66 +++++++-- .../Console/Commands/FetchNewFeaturesTest.php | 103 ++++++++++++- .../Livewire/Other/NewFeatureBannerTest.php | 112 ++++++++++++-- 4 files changed, 351 insertions(+), 68 deletions(-) diff --git a/app/Console/Commands/FetchNewFeatures.php b/app/Console/Commands/FetchNewFeatures.php index bfa5c248..0a569d44 100644 --- a/app/Console/Commands/FetchNewFeatures.php +++ b/app/Console/Commands/FetchNewFeatures.php @@ -6,24 +6,42 @@ use Exception; use Illuminate\Console\Command; -use Illuminate\Contracts\Filesystem\FileNotFoundException; -use Illuminate\Http\Client\RequestException; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; use Symfony\Component\Console\Command\Command as CommandAlias; /** - * Command to fetch new features from GitHub or a local file and store them in cache. + * Fetches new features from GitHub or a local file and stores them in cache. + * + * This command retrieves feature information either from a remote GitHub repository + * or a local JSON file, validates the data, and stores the latest valid feature + * in the application's cache for a week. */ class FetchNewFeatures extends Command { + /** + * The console command signature. + * + * @var string + */ protected $signature = 'vanguard:fetch-new-features {--local : Fetch from local new_features.json file}'; + /** + * The console command description. + * + * @var string + */ protected $description = 'Fetch new features from GitHub or local file and store in cache'; /** - * Handle the command execution. + * Execute the console command. + * + * This method orchestrates the feature fetching process, including fetching, + * validating, and storing the latest feature if it's new. + * + * @return int The command exit code (0 for success, 1 for failure) */ public function handle(): int { @@ -31,26 +49,43 @@ public function handle(): int $features = $this->fetchFeatures(); if ($features === []) { - $this->error('No new features found.'); + $this->components->error('No new features found.'); + + return CommandAlias::SUCCESS; + } - return CommandAlias::FAILURE; + $latestFeature = $this->getLatestValidFeature($features); + + if ($latestFeature === null) { + $this->components->error('No valid features found.'); + + return CommandAlias::SUCCESS; + } + + if (! $this->isNewFeature($latestFeature)) { + $this->components->error('No new features to store.'); + + return CommandAlias::SUCCESS; } - $this->storeLatestFeature($features); - $this->info('New features fetched and stored successfully.'); + $this->storeLatestFeature($latestFeature); + $this->components->info('New features fetched and stored successfully.'); return CommandAlias::SUCCESS; } catch (Exception $e) { $this->error($e->getMessage()); + Log::error('Failed to fetch new features: ' . $e->getMessage()); return CommandAlias::FAILURE; } } /** - * Fetch features from either local or remote source. + * Fetch features from either remote or local source based on the --local option. * - * @return array> + * @return array> An array of feature data + * + * @throws Exception If fetching or parsing fails */ private function fetchFeatures(): array { @@ -60,28 +95,27 @@ private function fetchFeatures(): array /** * Fetch features from the remote GitHub repository. * - * @return array> + * @return array> An array of feature data * - * @throws Exception + * @throws Exception If the HTTP request fails or returns an unsuccessful status */ private function fetchRemote(): array { - try { - $response = Http::get('https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json'); - $response->throw(); + $response = Http::get('https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json'); - return $response->json(); - } catch (RequestException $e) { - throw new Exception("Failed to fetch new features from remote: {$e->getMessage()}", $e->getCode(), $e); + if (! $response->successful()) { + throw new Exception("Failed to fetch new features from remote: HTTP request returned status code {$response->status()}"); } + + return $response->json(); } /** - * Fetch features from the local file. + * Fetch features from the local new_features.json file. * - * @return array> + * @return array> An array of feature data * - * @throws Exception + * @throws Exception If the file is not found or cannot be parsed */ private function fetchLocal(): array { @@ -102,37 +136,65 @@ private function fetchLocal(): array } /** - * Store the latest feature in the cache if available. + * Get the latest valid feature from the fetched features. * - * @param array> $features - * - * @throws FileNotFoundException + * @param array> $features The array of fetched features + * @return array|null The latest valid feature or null if none found */ - private function storeLatestFeature(array $features): void + private function getLatestValidFeature(array $features): ?array { - if ($features === []) { - $this->info('No features to store.'); + $validFeatures = array_filter($features, [$this, 'isValidFeature']); - return; - } + return end($validFeatures) ?: null; + } - $latestFeature = end($features); - if ($latestFeature === false) { - $this->error('Failed to retrieve the latest feature.'); + /** + * Check if a feature is valid (has all required fields). + * + * @param array $feature The feature to validate + * @return bool True if the feature is valid, false otherwise + */ + private function isValidFeature(array $feature): bool + { + return isset($feature['title'], $feature['description'], $feature['version']); + } - return; - } + /** + * Check if the given feature is newer than the currently cached feature. + * + * @param array $feature The feature to check + * @return bool True if the feature is new, false otherwise + */ + private function isNewFeature(array $feature): bool + { + $cachedFeature = Cache::get('latest_feature'); + + return ! $cachedFeature || version_compare($feature['version'], $cachedFeature['version'], '>'); + } + /** + * Store the latest feature in the cache. + * + * This method adds the current application version to the feature data + * and ensures a GitHub URL is present before caching. + * + * @param array $latestFeature The feature to store + */ + private function storeLatestFeature(array $latestFeature): void + { $latestFeature['current_version'] = $this->getCurrentVersion(); $latestFeature['github_url'] ??= 'https://github.com/vanguardbackup/vanguard'; - Cache::put('latest_feature', $latestFeature, now()->addDay()); + Cache::put('latest_feature', $latestFeature, now()->addWeek()); } /** - * Get the current version from the VERSION file. + * Get the current version of the application. + * + * This method reads the version from a VERSION file in the project root. + * If the file doesn't exist, it returns '0.0.0' as a default version. * - * @throws FileNotFoundException + * @return string The current version or '0.0.0' if not found */ private function getCurrentVersion(): string { diff --git a/app/Livewire/Other/NewFeatureBanner.php b/app/Livewire/Other/NewFeatureBanner.php index daff2504..323ef621 100644 --- a/app/Livewire/Other/NewFeatureBanner.php +++ b/app/Livewire/Other/NewFeatureBanner.php @@ -6,17 +6,28 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Session; use Illuminate\View\View; use Livewire\Component; /** * Displays a banner for new features in the application. - * Fetches the latest feature from cache and allows users to dismiss it. + * + * This Livewire component fetches the latest feature from cache, + * displays it to the user, and allows them to dismiss it. The dismissal + * state is stored in the user's session to prevent the same feature + * from being shown repeatedly. */ class NewFeatureBanner extends Component { /** - * The latest feature to display in the banner. + * The session key used to store the dismissed feature version. + * + * @var string + */ + private const string SESSION_KEY = 'dismissed_feature_version'; + /** + * The latest feature to be displayed in the banner. * * @var array|null */ @@ -24,21 +35,19 @@ class NewFeatureBanner extends Component /** * Initialize the component state. + * + * This method is called when the component is first loaded. + * It loads the latest feature from cache if available and not dismissed. */ public function mount(): void { - $cachedFeature = Cache::get('latest_feature'); - - if (is_array($cachedFeature)) { - $this->latestFeature = $cachedFeature; - } elseif ($cachedFeature !== null) { - Log::warning('Unexpected data type for latest_feature in cache', ['type' => gettype($cachedFeature)]); - $this->latestFeature = null; - } + $this->loadLatestFeature(); } /** * Render the component. + * + * @return View The view that represents the component */ public function render(): View { @@ -46,11 +55,44 @@ public function render(): View } /** - * Dismiss the feature banner. + * Dismiss the currently displayed feature. + * + * This method is called when the user chooses to dismiss the feature banner. + * It stores the dismissed version in the session and clears the latestFeature property. */ public function dismiss(): void { - $this->latestFeature = null; + if ($this->latestFeature) { + Session::put(self::SESSION_KEY, $this->latestFeature['version'] ?? 'unknown'); + $this->latestFeature = null; + } $this->dispatch('featureDismissed'); } + + /** + * Load the latest feature from cache if it hasn't been dismissed. + * + * This method checks the cache for the latest feature and compares it + * against the dismissed version stored in the session. If the feature + * is new or hasn't been dismissed, it's loaded into the component state. + */ + private function loadLatestFeature(): void + { + $cachedFeature = Cache::get('latest_feature'); + + if (! is_array($cachedFeature)) { + if ($cachedFeature !== null) { + Log::warning('Unexpected data type for latest_feature in cache', ['type' => gettype($cachedFeature)]); + } + + return; + } + + $dismissedVersion = Session::get(self::SESSION_KEY); + $currentVersion = $cachedFeature['version'] ?? 'unknown'; + + if ($dismissedVersion !== $currentVersion) { + $this->latestFeature = $cachedFeature; + } + } } diff --git a/tests/Feature/Console/Commands/FetchNewFeaturesTest.php b/tests/Feature/Console/Commands/FetchNewFeaturesTest.php index c10876fc..82d9a43b 100644 --- a/tests/Feature/Console/Commands/FetchNewFeaturesTest.php +++ b/tests/Feature/Console/Commands/FetchNewFeaturesTest.php @@ -28,7 +28,7 @@ $this->artisan(FetchNewFeatures::class) ->assertSuccessful() - ->expectsOutput('New features fetched and stored successfully.'); + ->expectsOutputToContain('New features fetched and stored successfully.'); expect(Cache::get('latest_feature'))->toBe([ 'title' => 'New Feature', @@ -45,8 +45,8 @@ ]); $this->artisan(FetchNewFeatures::class) - ->assertFailed() - ->expectsOutput('No new features found.'); + ->assertSuccessful() + ->expectsOutputToContain('No new features found.'); expect(Cache::get('latest_feature'))->toBeNull(); }); @@ -80,7 +80,7 @@ $this->artisan(FetchNewFeatures::class, ['--local' => true]) ->assertSuccessful() - ->expectsOutput('New features fetched and stored successfully.'); + ->expectsOutputToContain('New features fetched and stored successfully.'); expect(Cache::get('latest_feature'))->toBe([ 'title' => 'Local Feature', @@ -96,7 +96,7 @@ $this->artisan(FetchNewFeatures::class, ['--local' => true]) ->assertFailed() - ->expectsOutput('Local new_features.json file not found in project root.'); + ->expectsOutputToContain('Local new_features.json file not found in project root.'); expect(Cache::get('latest_feature'))->toBeNull(); }); @@ -107,7 +107,7 @@ $this->artisan(FetchNewFeatures::class, ['--local' => true]) ->assertFailed() - ->expectsOutput('Failed to parse local new_features.json file.'); + ->expectsOutputToContain('Failed to parse local new_features.json file.'); expect(Cache::get('latest_feature'))->toBeNull(); }); @@ -128,7 +128,96 @@ $this->artisan(FetchNewFeatures::class) ->assertSuccessful() - ->expectsOutput('New features fetched and stored successfully.'); + ->expectsOutputToContain('New features fetched and stored successfully.'); expect(Cache::get('latest_feature')['current_version'])->toBe('0.0.0'); }); + +it('does not update cache if fetched version is older', function (): void { + Cache::put('latest_feature', [ + 'title' => 'Existing Feature', + 'description' => 'This is an existing feature', + 'version' => '1.2.0', + 'github_url' => 'https://github.com/vanguardbackup/vanguard/releases/tag/1.2.0', + 'current_version' => '1.1.0', + ], now()->addWeek()); + + Http::fake([ + 'https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json' => Http::response([ + [ + 'title' => 'Older Feature', + 'description' => 'This is an older feature', + 'version' => '1.1.0', + 'github_url' => 'https://github.com/vanguardbackup/vanguard/releases/tag/1.1.0', + ], + ], 200), + ]); + + File::shouldReceive('exists')->with(base_path('VERSION'))->andReturn(true); + File::shouldReceive('get')->with(base_path('VERSION'))->andReturn('1.1.0'); + + $this->artisan(FetchNewFeatures::class) + ->assertSuccessful() + ->expectsOutputToContain('No new features to store.'); + + expect(Cache::get('latest_feature')['version'])->toBe('1.2.0'); +}); + +it('logs error when exception occurs', function (): void { + Http::fake([ + 'https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json' => Http::response(null, 500), + ]); + + Log::shouldReceive('error')->once()->with('Failed to fetch new features: Failed to fetch new features from remote: HTTP request returned status code 500'); + + $this->artisan(FetchNewFeatures::class) + ->assertFailed() + ->expectsOutputToContain('Failed to fetch new features from remote: HTTP request returned status code 500'); +}); + +it('handles invalid feature data', function (): void { + Http::fake([ + 'https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json' => Http::response([ + [ + 'title' => 'Invalid Feature', + // missing 'description' and 'version' + ], + ], 200), + ]); + + $this->artisan(FetchNewFeatures::class) + ->assertSuccessful() + ->expectsOutputToContain('No valid features found.'); + + expect(Cache::get('latest_feature'))->toBeNull(); +}); + +it('updates cache when fetched version is newer', function (): void { + Cache::put('latest_feature', [ + 'title' => 'Existing Feature', + 'description' => 'This is an existing feature', + 'version' => '1.1.0', + 'github_url' => 'https://github.com/vanguardbackup/vanguard/releases/tag/1.1.0', + 'current_version' => '1.0.0', + ], now()->addWeek()); + + Http::fake([ + 'https://raw.githubusercontent.com/vanguardbackup/vanguard/main/new_features.json' => Http::response([ + [ + 'title' => 'Newer Feature', + 'description' => 'This is a newer feature', + 'version' => '1.2.0', + 'github_url' => 'https://github.com/vanguardbackup/vanguard/releases/tag/1.2.0', + ], + ], 200), + ]); + + File::shouldReceive('exists')->with(base_path('VERSION'))->andReturn(true); + File::shouldReceive('get')->with(base_path('VERSION'))->andReturn('1.1.0'); + + $this->artisan(FetchNewFeatures::class) + ->assertSuccessful() + ->expectsOutputToContain('New features fetched and stored successfully.'); + + expect(Cache::get('latest_feature')['version'])->toBe('1.2.0'); +}); diff --git a/tests/Feature/Livewire/Other/NewFeatureBannerTest.php b/tests/Feature/Livewire/Other/NewFeatureBannerTest.php index e30e4aa5..021e4118 100644 --- a/tests/Feature/Livewire/Other/NewFeatureBannerTest.php +++ b/tests/Feature/Livewire/Other/NewFeatureBannerTest.php @@ -4,14 +4,20 @@ use App\Livewire\Other\NewFeatureBanner; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Session; +use Livewire\Livewire; it('shows the latest feature when available', function (): void { - $feature = ['title' => 'New Feature', 'description' => 'This is a new feature']; + $feature = [ + 'title' => 'New Feature', + 'description' => 'This is a new feature', + 'version' => '1.0.0', + ]; Cache::put('latest_feature', $feature, now()->addHour()); - $component = Livewire::test(NewFeatureBanner::class); - - $component->assertSee('New Feature') + Livewire::test(NewFeatureBanner::class) + ->assertSee('New Feature') ->assertSee('This is a new feature') ->assertSee('Dismiss'); }); @@ -19,20 +25,104 @@ it('hides the banner when no feature is available', function (): void { Cache::forget('latest_feature'); - $component = Livewire::test(NewFeatureBanner::class); - - $component->assertDontSee('New Feature') + Livewire::test(NewFeatureBanner::class) + ->assertDontSee('New Feature') ->assertDontSee('Dismiss'); }); it('dismisses the feature when clicked', function (): void { - $feature = ['title' => 'New Feature', 'description' => 'This is a new feature']; + $feature = [ + 'title' => 'New Feature', + 'description' => 'This is a new feature', + 'version' => '1.0.0', + ]; Cache::put('latest_feature', $feature, now()->addHour()); - $component = Livewire::test(NewFeatureBanner::class); - - $component->call('dismiss') + Livewire::test(NewFeatureBanner::class) + ->call('dismiss') ->assertDispatched('featureDismissed') ->assertDontSee('New Feature') ->assertDontSee('Dismiss'); + + expect(Session::get('dismissed_feature_version'))->toBe('1.0.0'); +}); + +it('does not show dismissed feature on subsequent loads', function (): void { + $feature = [ + 'title' => 'New Feature', + 'description' => 'This is a new feature', + 'version' => '1.0.0', + ]; + Cache::put('latest_feature', $feature, now()->addHour()); + Session::put('dismissed_feature_version', '1.0.0'); + + Livewire::test(NewFeatureBanner::class) + ->assertDontSee('New Feature') + ->assertDontSee('Dismiss'); +}); + +it('shows new feature even if previous version was dismissed', function (): void { + $feature = [ + 'title' => 'Newer Feature', + 'description' => 'This is a newer feature', + 'version' => '1.1.0', + ]; + Cache::put('latest_feature', $feature, now()->addHour()); + Session::put('dismissed_feature_version', '1.0.0'); + + Livewire::test(NewFeatureBanner::class) + ->assertSee('Newer Feature') + ->assertSee('This is a newer feature') + ->assertSee('Dismiss'); +}); + +it('logs a warning when cache contains unexpected data type', function (): void { + Cache::put('latest_feature', 'not an array', now()->addHour()); + + Log::shouldReceive('warning') + ->once() + ->with('Unexpected data type for latest_feature in cache', ['type' => 'string']); + + Livewire::test(NewFeatureBanner::class); +}); + +it('handles missing version in feature data', function (): void { + $feature = [ + 'title' => 'Incomplete Feature', + 'description' => 'This feature is missing a version', + ]; + Cache::put('latest_feature', $feature, now()->addHour()); + + Livewire::test(NewFeatureBanner::class) + ->assertSee('Incomplete Feature') + ->assertSee('This feature is missing a version') + ->assertSee('Dismiss'); + + Livewire::test(NewFeatureBanner::class) + ->call('dismiss'); + + expect(Session::get('dismissed_feature_version'))->toBe('unknown'); +}); + +it('does not show banner when dismissed version is unknown', function (): void { + $feature = [ + 'title' => 'Another Incomplete Feature', + 'description' => 'This feature is also missing a version', + ]; + Cache::put('latest_feature', $feature, now()->addHour()); + Session::put('dismissed_feature_version', 'unknown'); + + Livewire::test(NewFeatureBanner::class) + ->assertDontSee('Another Incomplete Feature') + ->assertDontSee('This feature is also missing a version') + ->assertDontSee('Dismiss'); +}); + +it('does not show banner when session is empty but cache is also empty', function (): void { + Cache::forget('latest_feature'); + Session::forget('dismissed_feature_version'); + + Livewire::test(NewFeatureBanner::class) + ->assertDontSee('New Feature') + ->assertDontSee('Dismiss'); });