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

Dashboard - Exclude Non Funding Flow Initiatives #248

Closed dan-tang-ssd closed 8 months ago

dan-tang-ssd commented 8 months ago

This PR is submitted for fix #244.

This PR contains one change:

  1. Dashboard, to exclude initiatives in portfolios that are not contributing to funding flow

In dashboard, we get initatives regardless whether it's portfolio is contributing to funding flow or not. I assume it will be the default way for getting initiatives in dashboard for comparison. Therefore I added a condition in stored procedure related SELECT SQL directly. If we add a new filter in dashboard Vue component, user will need to select it everytime.

Um... probably we can add a new filter selection box, with 3 options:

  1. Contribute to funding flow
  2. Not contribute to funding flow
  3. All

I am not quite sure whether it will increase complexity and create possible confusion. So... I stick with the current approach first.

what-the-diff[bot] commented 8 months ago

PR Summary

dan-tang-ssd commented 8 months ago

Performed testing in local env, with positive testing result.

BEFORE program change:

AFTER program change:


Test Case in local env:

  1. Confirm all existing portfolios are not contriubuting to funding flow
  2. Create two new institutions: Company A, Company B
  3. Create two portfolio for each newly created institution. One portfolio is contributing to funding flow, another portfolio is not contributing to funding flow
  4. Add one initiative in four new portfolios
  5. For each newly created initiative, assess red flag, assess principle (with different marks for each initiative to ease identification later one)

Company A

Company B

dan-tang-ssd commented 8 months ago

Screen shots BEFORE program change, all initiatives are included:


Company A

image

image

image


Company B

image

image

image

dan-tang-ssd commented 8 months ago

Screen shots AFTER program change, only initiatives in portfolios that are contributing to funding flow are included.

All previously created portfolios are not contributing to funding flow. There are only two newly create portfolios are contributing to funding flow. Each portfolio has only one initiative.


Company A

image

image

image


Company B

image

image

image

dan-tang-ssd commented 8 months ago

During my testing, I notice that in "Summary of Initiatives", we show a new line for "Failed at least one red flag". However, there is no corresponding help_text_entries record for it. So it is now showing nothing when mouseover it.

I have created a new record for it in local database via TablePlus. May I seek your advice for the best practice to handle this in live env?

We have below possible options, each with pros and cons, it seems there is no best choice here yet...

  1. Create migration file to create new record, it will be executed automatically during live env deployment. (A bit odd as migration file is normally for database structure, not for database records)
  2. Create a seeder file. After live env deployment, remote to server and run seeder file manually. (A bit risky as it is very easy to run other seeder files too, which may cause serious issue in live env...)
  3. After live env deployment, manually add new record in live database via TablePlus (Manual procedure required and it may be easy to forget. Related record not included in Github)

image

image

dave-mills commented 8 months ago

We have below possible options, each with pros and cons, it seems there is no best choice here yet...

You're right - there's no clear winner here for the options. I guess that's a limitation of having these strings in the database. A possible long-term option would be to make these help strings a json file, and then they can be stored and updated in the codebase. For now, I think probably options 1 or 3 are best - as you say, running Seeders on the live database is quite risky!

dave-mills commented 8 months ago

For the change - I think changing the SQL procedure is a sensible solution. I think only the others should be automatically filtered in this way. For now, we should continue to show all the initiatives of the current institution, even ones in portfolios that aren't contributing to the global analysis. Otherwise I think it will be confusing to some people ("I put in 5 initiatives and don't see any of them on the dashboard..."). Users can still filter by portfolio, so if they have a portfolio of tests and a "real" portfolio they can still filter out the tests manually.

Thanks!

dan-tang-ssd commented 8 months ago

Revised stored procedure as suggested.


Screen shot:

BEFORE: image

AFTER: image

dave-mills commented 8 months ago

Nice - thanks!