Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add minimum and maximum commitments total filter to capital projects endpoint #399

Open
7 tasks
TylerMatteo opened this issue Jan 13, 2025 · 12 comments · May be fixed by #423
Open
7 tasks

Add minimum and maximum commitments total filter to capital projects endpoint #399

TylerMatteo opened this issue Jan 13, 2025 · 12 comments · May be fixed by #423
Assignees

Comments

@TylerMatteo
Copy link
Collaborator

TylerMatteo commented Jan 13, 2025

Acceptance Criteria

  • capital-projects endpoint has two optional (but not nullable) query parameters to filter projects based on the funding total
    • commitmentsTotalMin, for a minimum total of funding
    • commitmentsTotalMax, for a maximum total of funding
  • The parameters represent USD (with no multiplier like "Millions")
  • The query parameters may be used together or on their own
    • When only commitmentsTotalMin is used, all returned projects must have total funding above or equal to the minimum
    • When only commitmentsTotalMax is used, all returned projects must have total funding below or equal to the maximum
    • When both parameters are used together, all project must have total funding within the range, inclusive.
  • New query parameters are reflected in OpenAPI spec (regenerate files as needed)
  • Add unit and e2e tests for new query parameters
    • Capital project repository mock should be updated to add "commitmentsTotal" filtering criteria
    • A service test should ensure that the capital project service is successfully passing the 'commitmentsTotalMin` parameter to the repository
    • A service test should ensure that the capital project service is successfully passing the 'commitmentsTotalMax` parameter to the repository
    • A service test should ensure the capital project service throws an error when the maximum is greater than the minimum
    • An e2e test should ensure the endpoint 200s and successfully returns a list of capital projects when filtering by a valid commitmentsTotalMin
    • An e2e test should ensure the endpoint 400s when filtering by an invalid commitmentsTotalMin
    • An e2e test should ensure the endpoint 200s and successfully returns a list of capital projects when filtering by a valid commitmentsTotalMax
    • An e2e test should ensure the endpoint 400s when filtering by an invalid commitmentsTotalMax
    • An e2e test should ensure the endpoint 400s when filtering by a commitmentsTotalMax higher than a commitmentsTotalMin

Validations

  • Return a 400 (invalid request parameter exception) if either provided value has more than two digits after the decimal place or otherwise does not reflect a valid US dollar value
  • Return a 400 (invalid request parameter exception) if both max and min are provided, but max is less than the min
@TangoYankee
Copy link
Member

How do we want to filter the mvts? Right now, the server doesn't return the total values as a property

@TylerMatteo TylerMatteo changed the title Add maximum project amount filter to capital projects endpoints Add minimum and maximum project amount filter to capital projects endpoint Jan 28, 2025
@TangoYankee TangoYankee changed the title Add minimum and maximum project amount filter to capital projects endpoint Add minimum and maximum commitments total filter to capital projects endpoint Jan 30, 2025
@TylerMatteo
Copy link
Collaborator Author

How do we want to filter the mvts? Right now, the server doesn't return the total values as a property

Good catch. I was thinking we would filter them on the front end via Deck's DataFilterExtension which would require adding the total to the tiles. This means we have to total up the projects' totals in the SQL query for the tile and return more data in the tiles, but I think that trade off beats doing the filtering server side and re-rendering all of the projects. I would make a separate issue for adding that field to the tiles.

@TangoYankee
Copy link
Member

The acceptance criteria read like the range is exclusive of the specified amount. However, we should make the range inclusive of the amounts.

@TangoYankee
Copy link
Member

Something to be careful of:

If the user has specified an "agency budget", then simply totaling the commitment values may give the amount for the commitments sponsored by that budget. However, I believe the intention is to filter based on all commitments, regardless of agency budget. In which case, we may need a common table expression (CTE) where we find the commitment amounts per capital project. Then, we select capital projects from that CTE.

@TylerMatteo
Copy link
Collaborator Author

I also took the requirements to mean it should total all commitments associated with any given project, regardless of agency budgets. That's the same logic used to create the commitmentsTotal field for each project, correct?

@TangoYankee
Copy link
Member

I also took the requirements to mean it should total all commitments associated with any given project, regardless of agency budgets. That's the same logic used to create the commitmentsTotal field for each project, correct?

correct

@TylerMatteo
Copy link
Collaborator Author

@TangoYankee the AC look good to me, but let me know if I'm missing anything. I'll make a separate issue for updating the tiles, this one will focus on the regular endpoint. @dhochbaum-dcp - pulling this into the sprint for you to pick up if you need work.

@TangoYankee
Copy link
Member

Before we get too far, do @dhochbaum-dcp, @horatiorosa , or @pratishta have objections, suggestions, or questions?

@dhochbaum-dcp
Copy link
Contributor

@TylerMatteo @TangoYankee I edited some grammar.

The validations don't specify, should commitmentsTotalMin=INVALID return a 400?

I think some more detail should be added to the Acceptance Criteria regarding tests, unless those are truly the criteria you'll accept.

@horatiorosa
Copy link
Contributor

@TangoYankee @TylerMatteo

for this validation
Return a 400 (invalid request parameter exception) if both max and min are provided, but max is less than or equal to the min

If the user only want to see capital projects whose commitment is a specific amount (say 1,000,000), they won't be able to do that?

@TangoYankee
Copy link
Member

I updated the validation criteria descriptions and specified test cases

@TangoYankee
Copy link
Member

If you want the fun of solving the SQL yourself, ignore the snippet below. If you don't want to spend brain cycles figuring out the CTE, here's some sql that consolidates the commitmentsTotal filter with the other filters (It still needs to be translated into drizzle):

WITH capital_project_summary AS (
	SELECT
		capital_project.managing_code,
		capital_project.id AS project_id,
		capital_project.description AS description,
		capital_project.managing_agency AS managing_agency,
		capital_project.max_date AS max_date,
		capital_project.min_date AS min_date,
		capital_project.category AS category,
	    ARRAY_AGG(DISTINCT capital_commitment.budget_line_code) AS agency_budgets,
		SUM(capital_commitment_fund.value) AS commitments_total,
		capital_project.li_ft_m_poly AS li_ft_m_poly,
		capital_project.li_ft_m_pnt AS li_ft_m_pnt
	FROM
		capital_project
	LEFT JOIN capital_commitment 
		ON capital_commitment.capital_project_id = capital_project.id
		AND capital_commitment.managing_code = capital_project.managing_code
	LEFT JOIN capital_commitment_fund
		ON capital_commitment_fund.capital_commitment_id = capital_commitment.id
	WHERE
		capital_commitment_fund.capital_fund_category = 'total'
	GROUP BY
		capital_project.managing_code,
		capital_project.id,
		capital_project.description,
		capital_project.managing_agency,
		capital_project.max_date,
		capital_project.min_date,
		capital_project.category
)
SELECT 
	capital_project_summary.managing_code,
	capital_project_summary.project_id,
	capital_project_summary.description,
	capital_project_summary.managing_agency,
	capital_project_summary.max_date,
	capital_project_summary.min_date,
	capital_project_summary.category
FROM capital_project_summary
LEFT JOIN city_council_district
	ON ST_INTERSECTS(city_council_district.li_ft, capital_project_summary.li_ft_m_poly)
	OR ST_INTERSECTS(city_council_district.li_ft, capital_project_summary.li_ft_m_pnt)
LEFT JOIN community_district
	ON ST_INTERSECTS(community_district.li_ft, capital_project_summary.li_ft_m_poly)
	OR ST_INTERSECTS(community_district.li_ft, capital_project_summary.li_ft_m_pnt)
WHERE
	city_council_district.id = '1'
	AND community_district.borough_id = '1'
	AND community_district.id = '01'
	AND capital_project_summary.managing_agency = 'DOT'
	AND capital_project_summary.commitments_total <= 15000
	AND capital_project_summary.commitments_total >= 15000
	AND 'HB' = ANY(capital_project_summary.agency_budgets)
ORDER BY
	capital_project_summary.managing_code,
	capital_project_summary.project_id;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants