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

Support Multiple Assessments #46

Closed dan-tang-ssd closed 1 year ago

dan-tang-ssd commented 1 year ago

This PR is submitted for supporting one project with multiple assessments.

Please note that it is not yet ready for review, this PR serves the purpose of progress update.

dan-tang-ssd commented 1 year ago

Hi Dave,

Um... your proposed way is quicker if the develop knows the exsiting code very well. For me... I used my way because I can make changes one by one by applying "Divide and Conquer" technique. If I change database table name and column names directly, most features in Project CRDU will not work. This may be longer for me as it will be many different places need changes, and I do not have a clear overview for how much to change...

By the way, I think it is around 70% completed in Project CRUD. I just realised that there are some other codes related to Project model function principleProject() in OrganisationController. It looks like they are not being used yet. Can we simply comment them first?

Below please find my status update FYI:

Items completed:

TODO:

dave-mills commented 1 year ago

Thanks for the status update.

I just realised that there are some other codes related to Project model function principleProject() in OrganisationController. It looks like they are not being used yet. Can we simply comment them first?

If you mean the scoreTags1, scoreTags2 etc, then those are actually used to populate the CRUD::field('scoreTags' . $principle->id) fields in the ProjectCrudController. (They're not explicitly referenced so your IDE probably shows them as un-used). It's a rough hack I added just to make the original Proof of Concept work as quickly as possible, but it relies on the Principles always having ids of 1 - 13. Ideally we should refactor those functions to generalise. That's on my TODO list for adding the Custom Assessment Criteria (issue #40), as I think it'll be relevant there.

dan-tang-ssd commented 1 year ago

Hi Dave,

Just a status update:

Completed:


Performed testing:


TODO:

dan-tang-ssd commented 1 year ago

To confirm, about the remaining TODO items you listed:

TODO:

  • Project CRUD, list view, Filter by Status, check for latest assessment instead of project

It looks like this one is complete?

Yes, this one is completed.


  • Remove column projects.assessment_status
  • Remove unnecessary functions in Project model
  • Rename tables principle_project_score_tag and custom_score_tags, update column name

And these ones aren't. Are you waiting to finalise this feature before removing these? I would really like these things tidied before we consider this feature complete + ready to merge.

Yes, they are not completed yet.

  • Remove column projects.assessment_status

I will work on it now.

  • Remove unnecessary functions in Project model

Tried several times before. There are other classes calling these functions. I will try to change to call these functions in Assessment model.

  • Rename tables principle_project_score_tag and custom_score_tags, update column name

Um... score tags are more related to principles. Will you handle them when you revise the curent design of principles and score tags?


The main question I have left here is: how will a user start a new assessment of a project? E.g., Project A has 1 completed assessment. How will a user create a 2nd assessment?

Oh yes. Um... maybe.... do you think we can add a Re-Assess button in CRUD list view?

dave-mills commented 1 year ago

Great :)

Um... score tags are more related to principles. Will you handle them when you revise the curent design of principles and score tags?

I'll handle any bugs and fixes when looking at the Principles + custom assessment criteria, but I think changing the relationships so that ScoreTag and CustomScoreTag are linked to assessments and not projects is a vital part of extracting the "assessment" functionality out of the "project" model, which is what this feature is doing.

Um... maybe.... do you think we can add a Re-Assess button in CRUD list view?

Sounds like a good, clear approach.

dan-tang-ssd commented 1 year ago

Completed:

Issue:

dave-mills commented 1 year ago

Thanks Dan.

custom_score_tags records of current assessment will be deleted after saving assessment every time, not sure whether it is related to AssessOperation, which is using project record as current entry. (Should we change to use assessment record as current entry? and how...?)

It looks like the items are being saved correctly, but then not being brought back into the form. I think you're right - it's because the form is on the ProjectCrudController, so the base entity used to populate all the CRUD fields is the Project, when really we should be editing the Assessment instead.

I'll take a look. It might be a case where we can move most of the Assessment operation over to an AssessmentCrudController to fix this issue.

dave-mills commented 1 year ago

@dan-tang-ssd. I think a logical appraoch here is to move the Redline and Assess Operation stuff over to a new AssessmentCrudController - Given we're no longer interacting with the Project model during those activities.

I've just done that, and it looks to have fixed the issue where score tags / custom-score-tags were not being reloaded into the form, because the Backpack forms now use the Assessment as the $entry.

To hook it up, I modified the save options available, so now by default the user is directed back to the project list view after saving redlines or finishing the assessment. I also tweaked the action buttons on the project list view slightly. I'll want to mofidy the order of the buttons and exactly how they look, but we should do that in the Layout / UI / UX review branch.

What do you think?

dan-tang-ssd commented 1 year ago

What do you think?

It sounds great, :-)