Skip to content

Conversation

nolanpro
Copy link
Contributor

@nolanpro nolanpro commented Aug 23, 2018

#537

In addition to adding the basic UI and crud stuff, this also refactors and cleans up a bunch of things.

No jest tests yet since I'm looking into a babel transform issue and this

Copy link
Contributor

@velkymx velkymx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job. Few changes and we're good to go.

@@ -7,40 +7,49 @@
use ProcessMaker\Exception\ValidationException;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this manager completely. Move the validation rules into the model and the business logic into the controller.

@@ -22,13 +22,17 @@ class ProcessCategory extends Model
use ValidatingTrait;
use Uuid;

const STATUS_ACTIVE = 'ACTIVE';
const STATUS_INACTIVE = 'INACTIVE';

/**
* Validation rules.
*
* @var array $rules
*/
protected $rules = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the rules public so we can use them in the controller.

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;

class AddStatusToProcessCategoriesTable extends Migration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the current migration completely. Don't add an additional migration.

@velkymx
Copy link
Contributor

velkymx commented Aug 28, 2018

@nolanpro when editing a category the alert message shows "New Category Successfully Created" should say Category Updated Successfully

@nolanpro
Copy link
Contributor Author

Ok should be fixed

@velkymx velkymx merged commit 8f39883 into develop Aug 28, 2018
@velkymx velkymx deleted the feature/537 branch August 10, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants