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

Copy amount per PU for shapefiles and legacy [MRXN23-466] [MRXN23-467] #1559

Conversation

KevSanchez
Copy link
Collaborator

Substitute this line for a meaningful title for your changes

Overview

Please write a description. If the PR is hard to understand, provide a quick explanation of the code.

Designs

Link to the related design prototypes (if applicable)

Testing instructions

Please explain how to test the PR: ID of a dataset, steps to reach the feature, etc.

Feature relevant tickets

Link to the related task manager tickets


Checklist before submitting

  • Meaningful commits and code rebased on develop.
  • If this PR adds feature that should be tested for regressions when
    deploying to staging/production, please add brief testing instructions
    to the deploy checklist (docs/deployment-checklist.md)
  • Update CHANGELOG file

@KevSanchez KevSanchez requested a review from hotzevzl October 25, 2023 13:20
@vercel
Copy link

vercel bot commented Oct 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marxan ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 26, 2023 11:07am

@KevSanchez KevSanchez changed the title Feature/api/mrxn23 466 467 copy amount per pu for shapefiles and legacy Copy amount per PU for shapefiles and legacy [MRXN23-466] [MRXN23-467] Oct 25, 2023
Comment on lines 386 to 419
private async updateAmountMinMaxForAPIFeatures(
apiEntityManager: EntityManager,
featureIds: string[],
): Promise<void> {
this.logger.log(`Saving min and max amounts for new features...`);

const minAndMaxAmountsForFeatures = await this.geoprocessingEntityManager
.createQueryBuilder()
.select('feature_id', 'id')
.addSelect('MIN(amount)', 'amountMin')
.addSelect('MAX(amount)', 'amountMax')
.from('feature_amounts_per_planning_unit', 'fappu')
.where('fappu.feature_id IN (:...featureIds)', { featureIds })
.groupBy('fappu.feature_id')
.getRawMany();

const minMaxSqlValueStringForFeatures = minAndMaxAmountsForFeatures
.map(
(feature) =>
`(uuid('${feature.id}'), ${feature.amountMin}, ${feature.amountMax})`,
)
.join(', ');

const query = `
update features set
amount_min = minmax.min,
amount_max = minmax.max
from (
values
${minMaxSqlValueStringForFeatures}
) as minmax(feature_id, min, max)
where features.id = minmax.feature_id;`;
await apiEntityManager.query(query);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok to keep this in this PR - in a later one, I suggest to use a single instance of this code, after adding a parameter to allow to pass in an apiEntityManager and removing the reliance on any entity managers injected in the parent class, so that we can remove duplicated code... though if moving this to an existing lib (for example libs/geofeature-calculations) turns out to be even just slightly more than banal, duplication be it and let's move on with life.

@hotzevzl hotzevzl closed this Oct 26, 2023
@hotzevzl hotzevzl force-pushed the feature/api/MRXN23-466-467-copy-amount-per-pu-for-shapefiles-and-legacy branch from 0f206c6 to 2ac1fc3 Compare October 26, 2023 11:03
@hotzevzl hotzevzl deleted the feature/api/MRXN23-466-467-copy-amount-per-pu-for-shapefiles-and-legacy branch October 26, 2023 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants