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

Review init review #210

Closed dave-mills closed 1 year ago

dave-mills commented 1 year ago

PR to fix #197:

Also added a minor fix so that the blade help-text-link can now also act as a popover.

dan-tang-ssd commented 1 year ago

Pulled latest code, executed migration file, performed testing in local env.

Most testings are performed with positve result, just the last testing with question about the condition to show or hide principle summary.


It shows budget in two currencies when initiative currency is not same as institution currency. Score and rating looks good and clear. image

It shows budget in one currency when initiative currency is same as institution currency. image


In the generated PDF file, initiative basic info, spider chart, score and rating are always in page 1. image

Principle summary is always in page 2. image


For initiative not yet start red flag assessment, principle summary is not showed. image

image

For initiative finished all assessments, then assess red flags and failed one red flag, principle summary is still showed. Question: Should we also check red flag status to determine whether to show principle summary or not? image

image

dave-mills commented 1 year ago

Question: Should we also check red flag status to determine whether to show principle summary or not?

Good idea. I've just added that check in.

On the rendering error on other branches, I suggest we merge this one in, then merge dev branch into the other branches and it should be fine. It'll be a caching thing where the blade component is looking for the class added in this PR, (it's odd that once you've added a class, it always looks for it. Some caching thing that I don't understand, I assume).