Skip to content

Commit

Permalink
[AIDAPP-288] & [AIDAPP-274] & [AIDAPP-275]: Switch to class based fea…
Browse files Browse the repository at this point in the history
…ture flagging (#258)

* Switch to class based Feature Flags

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* Ensure Feature Flags are not purged

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* Remove the Portal View Count Feature

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* Update Feature Flagging docs

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* Add additional PR templates notes

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* Update some wording

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>

* chore: fix enforcement of copyright on all files

* chore: fix code style

---------

Signed-off-by: Kevin Ullyott <kevin.ullyott@canyongbs.com>
Co-authored-by: joelicatajr <joelicatajr@users.noreply.github.com>
  • Loading branch information
Orrison and joelicatajr authored Sep 27, 2024
1 parent d3b7df5 commit 03edbf0
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 107 deletions.
7 changes: 4 additions & 3 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
> If yes, please replace this block with a description of the deployment steps required and apply the `Deployment Steps` label to the PR.
> If no, please replace this block with "No".
### Are any Feature Flags Added?
### Are any Feature Flags and/or Data Migrations that can eventually be removed Added?

> Specifically Feature Flags to gate and control logic/flow changes to protect against database schema changes.
> Specifically Feature Flags to gate and control logic/flow changes to protect against database schema changes. And Data Migrations to provide some type of data fix that can be removed after they are run
>
> If yes, please list the name of the added Feature Flags below and apply the `Feature Flag Added` label to the PR.
> If yes to Feature Flags, please list the name of the added Feature Flag/s and apply the `Feature Flag Added` label to the PR.
> If yes to Data Migrations, please list the full relative file path of the added Data Migration/s.
> If no, please replace this block with "No".
_______________________________________________
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

namespace AidingApp\Portal\Http\Controllers\KnowledgeManagementPortal;

use Laravel\Pennant\Feature;
use Illuminate\Http\JsonResponse;
use App\Http\Controllers\Controller;
use AidingApp\KnowledgeBase\Models\KnowledgeBaseItem;
Expand All @@ -48,11 +47,7 @@ class KnowledgeManagementPortalArticleController extends Controller
{
public function show(KnowledgeBaseCategory $category, KnowledgeBaseItem $article): JsonResponse
{
$portalViewCountFlag = Feature::active('portal_view_count');

if ($portalViewCountFlag) {
$article->increment('portal_view_count');
}
$article->increment('portal_view_count');

return response()->json([
'category' => KnowledgeBaseCategoryData::from([
Expand All @@ -76,7 +71,6 @@ public function show(KnowledgeBaseCategory $category, KnowledgeBaseItem $article
->toArray(),
]),
'portal_view_count' => $article->portal_view_count,
'portal_view_count_flag' => $portalViewCountFlag,
]);
}
}
72 changes: 0 additions & 72 deletions app/Enums/FeatureFlag.php

This file was deleted.

Empty file added app/Features/.gitkeep
Empty file.
5 changes: 1 addition & 4 deletions app/Providers/AppServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@

use App\Models\Tenant;
use Sentry\State\Scope;
use App\Enums\FeatureFlag;
use App\Models\SystemUser;
use Laravel\Pennant\Feature;

Expand Down Expand Up @@ -118,8 +117,6 @@ public function boot(): void
return null;
});

collect(FeatureFlag::cases())->each(
fn (FeatureFlag $feature) => Feature::define($feature->value, $feature->definition())
);
Feature::discover();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,29 @@
</COPYRIGHT>
*/

namespace App\Support;

use Laravel\Pennant\Feature;
use Illuminate\Database\Migrations\Migration;

return new class () extends Migration {
public function up(): void
abstract class AbstractFeatureFlag
{
public static function active(): bool
{
return Feature::active(static::class);
}

public static function activate(): void
{
Feature::activate(static::class);
}

public static function deactivate(): void
{
Feature::activate('portal_view_count');
Feature::deactivate(static::class);
}

public function down(): void
public static function purge(): void
{
Feature::deactivate('portal_view_count');
Feature::purge(static::class);
}
};
}
2 changes: 1 addition & 1 deletion docs/data-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ When creating data migrations it is important to adhere to the following rules:
1. Do **NOT** use references to classes and facades that could be removed in the future. This includes Eloquent models. Currently, the only accepted classes to use within migrations are as follows:
- `Illuminate\Support\Facades\DB`
- Used to Interact with the database.
- `App\Enums\FeatureFlag`
- Any Feature Flag class located within the `App\Features` namespace extending `App\Support\AbstractFeatureFlag`
- To interact with feature flagging.
2. As mentioned above, ensure all possible SQL errors are captured and handled, such as `UniqueConstraintExeception`. Also ensure that any database change is surrounded by a transaction to prevent corruption of the query connection.
2. Ensure that the migration is idempotent. This means that the migration can be run multiple times without causing any issues. Tables and columns should be checked for the existence, SQL errors should be caught and properly handled, and data should be checked if it is in the expected state before it is changed, etc.
Expand Down
22 changes: 14 additions & 8 deletions docs/feature-flagging.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ The most common scenario is to allow our application to deploy with zero downtim
To prevent errors or other issues, we apply Feature Flags to areas that require the migrations to have already been completed. For example:

```php
if (FeatureFlag::SomeFeature->active()) {
if (FeatureFlagClass::active()) {
// The feature flag is active, so we know the database schema and data migrations have run, and the new logic is executed here.
} else {
// Old logic that does not require any changes from schema or data migrations is executed here.
Expand All @@ -20,36 +20,42 @@ if (FeatureFlag::SomeFeature->active()) {

Any change to the database schema or data that would cause issues if the code were accessed and executed before the migrations finished must have a Feature Flag to prevent issues from occurring. This can be done either by preventing the new code from running at all or by using the old code if it is not yet active.

New Feature Flags should be added as a case to the `App\Enums\FeatureFlag` ENUM. This ENUM contains many helpers for managing Feature Flags, such as activating, deactivating, and checking the status of a Feature Flag.
New Feature Flags should be generated by the command below, creating an class-based Feature Flag. This class will contain many helpers for managing Feature Flags, such as activating, deactivating, and checking the status of a Feature Flag from the base abstract class it extends. It will automatically be registered in the container and immediatly available for you to use in the areas of the application you require it.

```bash
php artisan pennant:feature [The name you want for your Feature Flag class like SomeFeature]
```

In most cases the Feature Flag class can be left how it is generated. It's `resolve` method will return `false` it cause it to be deactivated until specifically activated by a data migration, described below.

After you have created all your schema and data migrations, you should create a new data migration specifically to activate the Feature Flag. For example:

```php
use App\Enums\FeatureFlag;
use App\Features\SomeFeature;
use Illuminate\Database\Migrations\Migration;

return new class () extends Migration {
public function up(): void
{
FeatureFlag::SisIntegrationSettings->activate();
SomeFeature::activate();
}

public function down(): void
{
FeatureFlag::SisIntegrationSettings->deactivate();
SomeFeature::deactivate();
}
};
```

Optionally, you can choose to add a custom match in the definition of the `App\Enums\FeatureFlag` ENUM to activate the Feature Flag if certain conditions are met, such as checking if certain tables or columns exist, or if certain migrations have run, etc. However, you still MUST create the activation migration to ensure it gets activated, just in case someone executes code that checks the feature, parses the definition, and has marked it as deactivated.
Optionally, you can choose to add logic to `resolve` and return `true` to activate the Feature Flag if certain conditions are met, such as checking if certain tables or columns exist, or if certain migrations have run, etc. However, you still MUST create the activation migration to ensure it gets activated, just in case someone executes code that checks the feature, parses the definition, and has marked it as deactivated.

## Feature Flag cleanup

After the deployment containing your new changes and Feature Flag has gone out and successfully executed, there will generally be a task in the next release cycle to remove the Feature Flag.

To remove the Feature Flag, you should delete all references to its case, adjust any logic to work as if the Feature Flag were active, and delete any unneeded legacy code. You should then also delete the activation migration, and finally, remove the case from the `App\Enums\FeatureFlag` ENUM.
To remove the Feature Flag, you should delete all references to it, adjust any logic to work as if the Feature Flag were active, and delete any unneeded legacy code. You should then also delete the activation migration, and finally, delete the Feature Flag class.

The Feature Flag will be purged from the database automatically if it is no longer present in the ENUM.
The Feature Flag will be purged from the database automatically if it is no longer present.

### Additional Details

Expand Down
6 changes: 1 addition & 5 deletions portals/knowledge-management/src/Pages/ViewArticle.vue
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
article.value = response.data.article;
loading.value = false;
portalViewCount.value = response.data.portal_view_count;
portalViewCountFlag.value = response.data.portal_view_count_flag;
},
);
}
Expand All @@ -112,10 +111,7 @@
<div class="prose max-w-none">
<h1>{{ article.name }}</h1>
<div class="flex mb-4">
<div
v-if="portalViewCountFlag"
class="text-gray-500 flex items-center space-x-1 mr-2"
>
<div class="text-gray-500 flex items-center space-x-1 mr-2">
<EyeIcon class="h-4 w-4 flex-shrink-0" aria-hidden="true" />
<span class="text-xs">{{ portalViewCount }} Views</span>
</div>
Expand Down
13 changes: 13 additions & 0 deletions stubs/feature.stub
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

namespace {{ namespace }};

use App\Features\AbstractFeatureFlag;

class {{ class }} extends AbstractFeatureFlag
{
public function resolve(mixed $scope): mixed
{
return false;
}
}

0 comments on commit 03edbf0

Please sign in to comment.