stats4sd / aec_portfolio

A proof of concept for the AEC Consortium Project Management / Assessment System
GNU General Public License v3.0
0 stars 0 forks source link

Add duplicate initiative feature #273

Closed dave-mills closed 3 months ago

dave-mills commented 3 months ago

fixes #274.

This PR add the option for users to duplicate a project and all associated assessments/entries. This is to address times when a project gets an extension, and the user wants to associate the new budget with the date of the extension and not the date of the project start.

Duplicate Function

The duplicate function does the following:

For each new assessment:

The button is disabled using the same logic as the delete project option:

The duplicate function also checks that the user "can" create projects, based on the ProjectPoilicy class.

Other updates

This PR also fixes #279 - it adds "code" as a sort option on the Initiative List page.

dave-mills commented 3 months ago

Tested with fresh database - Quick video demos:

https://github.com/user-attachments/assets/83822332-c731-48a9-96eb-c871290cd043

dave-mills commented 3 months ago

Video showing "sort by code":

https://github.com/user-attachments/assets/cbdee564-3fbe-452c-93e7-3ad3007ea65a

dan-tang-ssd commented 3 months ago

I created a new institution, signed the agreement, create a portfolio and an initiative. Assessed red flag, principle and additional criteria.

The "Duplicate" button is disabled as expected:

  1. when agreement is not signed yet (update organisations record to pretend not yet signed)
  2. when user does not have permission to create initiative (login as institution member for testing)
dan-tang-ssd commented 3 months ago

After clicking "Duplicate" button, it does not take me to the cloned initiative Edit page. It keeps staying on the page, no new page showed...

I did a checking in local db, records copying has been done.

But I found error in laravel.log... It looks like complaining principle_assessment_score_tag.principle_assessment_id needs to have a default value...


Error message:

[2024-07-29 15:14:50] local.ERROR: SQLSTATE[HY000]: General error: 1364 Field 'principle_assessment_id' doesn't have a default value (Connection: mysql, SQL: insert into custom_score_tags (assessment_id, name, description, created_at, updated_at) values (739, descriptive name 3-1, ?, 2024-07-29 13:22:58, 2024-07-29 13:22:58)) {"userId":9,"exception":"[object] (Illuminate\Database\QueryException(code: HY000): SQLSTATE[HY000]: General error: 1364 Field 'principle_assessment_id' doesn't have a default value (Connection: mysql, SQL: insert into custom_score_tags (assessment_id, name, description, created_at, updated_at) values (739, descriptive name 3-1, ?, 2024-07-29 13:22:58, 2024-07-29 13:22:58)) at C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Connection.php:822) [stacktrace]

0 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Connection.php(776): Illuminate\Database\Connection->runQueryCallback('insert into `cu...', Array, Object(Closure))

1 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Connection.php(569): Illuminate\Database\Connection->run('insert into `cu...', Array, Object(Closure))

2 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Connection.php(533): Illuminate\Database\Connection->statement('insert into `cu...', Array)

3 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Query\Processors\Processor.php(32): Illuminate\Database\Connection->insert('insert into `cu...', Array)

4 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Query\Builder.php(3387): Illuminate\Database\Query\Processors\Processor->processInsertGetId(Object(Illuminate\Database\Query\Builder), 'insert into `cu...', Array, 'id')

5 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Builder.php(1968): Illuminate\Database\Query\Builder->insertGetId(Array, 'id')

6 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Model.php(1333): Illuminate\Database\Eloquent\Builder->__call('insertGetId', Array)

7 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Model.php(1298): Illuminate\Database\Eloquent\Model->insertAndSetId(Object(Illuminate\Database\Eloquent\Builder), Array)

8 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Model.php(1137): Illuminate\Database\Eloquent\Model->performInsert(Object(Illuminate\Database\Eloquent\Builder))

9 C:\public\aec\vendor\laravel\framework\src\Illuminate\Database\Eloquent\Relations\HasOneOrMany.php(342): Illuminate\Database\Eloquent\Model->save()

10 C:\public\aec\vendor\laravel\framework\src\Illuminate\Support\helpers.php(307): Illuminate\Database\Eloquent\Relations\HasOneOrMany->Illuminate\Database\Eloquent\Relations\{closure}(Object(App\Models\CustomScoreTag))


Screen shots:

image

dan-tang-ssd commented 3 months ago

When I refreshed the initiatives page, it shows the original project and the cloned project. But the additional criteria assessment is "Not Started" for the cloned initiative, while the original initiaitve is "Complete"...

Not quite sure whether it is related to the previously mentioned SQL error...


Questions:

  1. Will we have issue if two projects have the same project code?
  2. Will it be a good idea to add a suffix to project code to ensure it is unique?

Screen shot:

image

dan-tang-ssd commented 3 months ago

There is no error occurred when duplicating an initiative.

But the status for additional criteria is not "cloned"... It should be "Complete" instead of "Not Started"...


Screen shot:

image

dan-tang-ssd commented 3 months ago

I am checking the database records of the duplicated initiative.

It looks like below column value needs to be changed to the cloned initiative's assessment id:


We may consider to unset value for columns created_at and updated_at, so that database can fill in system date time when creating new records. If we copied created_at and updated_at of the original initiative to the cloned initiative. This may create confusion when we investigate database records in future.


Screen shots:

image

dave-mills commented 3 months ago

additional_criteria_custom_score_tags.assessment_id

If that's the case, it'll be the same for the custom_score_tags.assessment_id. I'll update.

dan-tang-ssd commented 3 months ago

When I refreshed the initiatives page, it shows the original project and the cloned project. But the additional criteria assessment is "Not Started" for the cloned initiative, while the original initiaitve is "Complete"...

Not quite sure whether it is related to the previously mentioned SQL error...

Questions:

1. Will we have issue if two projects have the same project code?

2. Will it be a good idea to add a suffix to project code to ensure it is unique?

Screen shot:

image

The status for additional criteria is cloned correctly now.

For project code, front end will do existence check on it. Let's front end to show error message if user has not updated project code. No change is required.

dan-tang-ssd commented 3 months ago

additional_criteria_custom_score_tags.assessment_id

If that's the case, it'll be the same for the custom_score_tags.assessment_id. I'll update.

Yes, you are right...

image

dan-tang-ssd commented 3 months ago

Just duplicate an initiative again, everything is fine. I think it is now good to merge.

dave-mills commented 3 months ago

For project code, front end will do existence check on it. Let's front end to show error message if user has not updated project code. No change is required.

Agreed. We can assume the user will update the project code and any other relevant information for the cloned project.

We may consider to unset value for columns created_at and updated_at, so that database can fill in system date time when creating new records. If we copied created_at and updated_at of the original initiative to the cloned initiative. This may create confusion when we investigate database records in future.

The project + assessment entries should currently have the correct timestamps, as we're using Laravel's replicate() method which handles that. (Each of the entries I have duplicated locally have different time stamps).

I do agree with you for the other entries - it's best to have accurate timestamps. I've updated it so that the 2 custom score tags updates do not push the previous timestamps. (Same with the principle_assessments, though locally I don't have any timestamps in that table. I don't consider that worth exploring now, so I will publish this as-is).

sentry-io[bot] commented 3 months ago

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎