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

Ajout de la restriction sur les potentiels doublons hors meme departement #478

Merged
merged 4 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions lib/api/address/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {checkDataFormat, dataValidationReportFrom, checkIdsIsUniq, checkIdsIsVacant, checkIdsIsAvailable, checkDataShema, checkIdsShema, checkIfCommonToponymsExist, checkIfDistrictsExist} from '../helper.js'
import {banID} from '../schema.js'
import {getCogFromDistrictID} from '../district/models.js'
import {getAddressesByFilters} from './models.js'
import {banAddressSchema} from './schema.js'

Expand Down Expand Up @@ -82,25 +83,45 @@ export const checkAddressesRequest = async (addresses, actionType) => {
return report
}

// Determines the slice length for a department based on the district ID,If the district ID is greater than '95', it returns 3, otherwise

const getDepartmentSliceLength = districtID => districtID > '95' ? 3 : 2
jbouhadoun marked this conversation as resolved.
Show resolved Hide resolved

export const getDeltaReport = async (addressIDsWithHash, districtID) => {
const idsToCreate = []
const idsToUpdate = []
const idsToDelete = []
const idsUnauthorized = []

// Get all existing addresses inside and outside the district from db
const addressIDs = addressIDsWithHash.map(({id}) => id)
const existingAddresses = await getAddressesByFilters({id: addressIDs}, ['id', 'meta', 'isActive'])
const existingAddressesMap = new Map(existingAddresses.map(({id, meta, isActive}) => [id, {hash: meta?.idfix?.hash, isActive}]))
const existingAddresses = await getAddressesByFilters({id: addressIDs}, ['id', 'meta', 'isActive', 'districtID'])
const existingAddressesMap = new Map(existingAddresses.map(({id, meta, isActive, districtID}) => [id, {hash: meta?.idfix?.hash, isActive, districtID}]))
const currentCog = await getCogFromDistrictID(districtID)

for (const {id, hash} of addressIDsWithHash) {
if (existingAddressesMap.has(id)) {
// The address is already existing in the db
const existingAddressIsActive = existingAddressesMap.get(id).isActive
const existingAddressHash = existingAddressesMap.get(id).hash
// If the address has a different hash we need to update it
// If the address is not active, it means that we need to reactivate it (even if the hash is the same)
if (existingAddressHash !== hash || !existingAddressIsActive) {
idsToUpdate.push(id)
const existingAddresseDistrictID = existingAddressesMap.get(id).districtID
// eslint-disable-next-line no-negated-condition
if (existingAddresseDistrictID !== districtID) {
// eslint-disable-next-line no-await-in-loop
const existingCog = await getCogFromDistrictID(existingAddresseDistrictID)
// Departement check

const sliceLength = getDepartmentSliceLength(districtID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const sliceLength = getDepartmentSliceLength(districtID)
const sliceLength = getDepartmentSliceLength(existingCog)

Copy link
Contributor

Choose a reason for hiding this comment

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

ou

Suggested change
const sliceLength = getDepartmentSliceLength(districtID)
const sliceLength = getDepartmentSliceLength(currentCog)

ce qui permet de le sortir de la boucle


if (currentCog.slice(0, sliceLength) !== existingCog.slice(0, sliceLength)) {
Comment on lines +112 to +114
Copy link
Contributor

Choose a reason for hiding this comment

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

préférer un getDepartmentCode(cog), peut être plus parlant (faire le slice dedans)

Copy link
Contributor

Choose a reason for hiding this comment

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

une fonction semblable existe dans lib/util/cog.cjs getCodeDepartement. Mais on ne veut peut être pas cette dépendance ici.

idsUnauthorized.push(id)
console.log(`Warning: addresse with ID ${id} belongs to a different district (current: ${currentCog}, existing: ${existingCog}). This update will be ignored as it already exists elsewhere.`)
}
} else { // The address is already existing in the db
const existingAddressIsActive = existingAddressesMap.get(id).isActive
const existingAddressHash = existingAddressesMap.get(id).hash
// If the address has a different hash we need to update it
// If the address is not active, it means that we need to reactivate it (even if the hash is the same)
if (existingAddressHash !== hash || !existingAddressIsActive) {
idsToUpdate.push(id)
}
}
} else {
// The address is not existing in the db
Expand All @@ -120,7 +141,7 @@ export const getDeltaReport = async (addressIDsWithHash, districtID) => {
}
}

return {idsToCreate, idsToUpdate, idsToDelete}
return {idsToCreate, idsToUpdate, idsToDelete, idsUnauthorized}
}

export const formatAddress = address => {
Expand Down
3 changes: 3 additions & 0 deletions lib/api/address/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ describe('getDeltaReport', () => {
idsToCreate: addressMock.map(({id}) => id),
idsToUpdate: [],
idsToDelete: bddAddressMock.map(({id}) => id),
idsUnauthorized: []
})
})
it('Should not return anything to update as hashes are equals', async () => {
Expand All @@ -185,6 +186,7 @@ describe('getDeltaReport', () => {
idsToCreate: [],
idsToUpdate: [],
idsToDelete: [],
idsUnauthorized: []
})
})
it('Should return only one id to update as hashes are different', async () => {
Expand All @@ -196,6 +198,7 @@ describe('getDeltaReport', () => {
idsToCreate: [],
idsToUpdate: [bddAddressMock[0].id],
idsToDelete: [],
idsUnauthorized: []
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Serait bien de rajouter un test idsUnauthorized, pour pour ce cas hors departement

})
40 changes: 31 additions & 9 deletions lib/api/common-toponym/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {checkDataFormat, dataValidationReportFrom, checkIdsIsUniq, checkIdsIsVacant, checkIdsIsAvailable, checkDataShema, checkIdsShema, checkIfDistrictsExist} from '../helper.js'
import {banID} from '../schema.js'
import {getCogFromDistrictID} from '../district/models.js'
import {getCommonToponymsByFilters} from './models.js'
import {banCommonToponymSchema} from './schema.js'

Expand Down Expand Up @@ -73,26 +74,47 @@ export const checkCommonToponymsRequest = async (commonToponyms, actionType) =>

return report
}
// Determines the slice length for a department based on the district ID,If the district ID is greater than '95', it returns 3, otherwise

const getDepartmentSliceLength = districtID => districtID > '95' ? 3 : 2

export const getDeltaReport = async (commonToponymIDsWithHash, districtID) => {
const idsToCreate = []
const idsToUpdate = []
const idsToDelete = []
const idsUnauthorized = []

// Get all existing common toponyms inside and outside the district from db
const commonToponymIDs = commonToponymIDsWithHash.map(({id}) => id)
const existingCommonToponyms = await getCommonToponymsByFilters({id: commonToponymIDs}, ['id', 'meta', 'isActive'])
const existingCommonToponymsMap = new Map(existingCommonToponyms.map(({id, meta, isActive}) => [id, {hash: meta?.idfix?.hash, isActive}]))
const existingCommonToponyms = await getCommonToponymsByFilters({id: commonToponymIDs}, ['id', 'meta', 'isActive', 'districtID'])
const existingCommonToponymsMap = new Map(existingCommonToponyms.map(({id, meta, isActive, districtID}) => [id, {hash: meta?.idfix?.hash, isActive, districtID}]))
const currentCog = await getCogFromDistrictID(districtID)

for (const {id, hash} of commonToponymIDsWithHash) {
if (existingCommonToponymsMap.has(id)) {
const existingCommonToponymDistrictID = existingCommonToponymsMap.get(id).districtID

// eslint-disable-next-line no-negated-condition
if (existingCommonToponymDistrictID !== districtID) {
// eslint-disable-next-line no-await-in-loop
const existingCog = await getCogFromDistrictID(existingCommonToponymDistrictID)
// Departement check

const sliceLength = getDepartmentSliceLength(districtID)
if (currentCog.slice(0, sliceLength) !== existingCog.slice(0, sliceLength)) {
idsUnauthorized.push(id)
console.log(`Warning: Common toponym with ID ${id} belongs to a different district (current: ${currentCog}, existing: ${existingCog}). This addition will be ignored as it already exists elsewhere.`)
}
} else {
// The common toponym is already existing in the db
const existingCommonToponymIsActive = existingCommonToponymsMap.get(id).isActive
const existingCommonToponymHash = existingCommonToponymsMap.get(id).hash
// If the common toponym has a different hash we need to update it
// If the common toponym is not active, it means that we need to reactivate it (even if the hash is the same)
if (existingCommonToponymHash !== hash || !existingCommonToponymIsActive) {
idsToUpdate.push(id)
const existingCommonToponymIsActive = existingCommonToponymsMap.get(id).isActive
const existingCommonToponymHash = existingCommonToponymsMap.get(id).hash
// If the common toponym has a different hash we need to update it
// If the common toponym is not active, it means that we need to reactivate it (even if the hash is the same)

if (existingCommonToponymHash !== hash || !existingCommonToponymIsActive) {
idsToUpdate.push(id)
}
}
} else {
// The common toponym is not existing in the db
Expand All @@ -112,7 +134,7 @@ export const getDeltaReport = async (commonToponymIDsWithHash, districtID) => {
}
}

return {idsToCreate, idsToUpdate, idsToDelete}
return {idsToCreate, idsToUpdate, idsToDelete, idsUnauthorized}
}

export const formatCommonToponym = commonToponym => {
Expand Down
3 changes: 3 additions & 0 deletions lib/api/common-toponym/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ describe('getDeltaReport', () => {
idsToCreate: commonToponymMock.map(({id}) => id),
idsToUpdate: [],
idsToDelete: bddCommonToponymMock.map(({id}) => id),
idsUnauthorized: []
})
})
it('Should not return anything to update as hashes are equals', async () => {
Expand All @@ -160,6 +161,7 @@ describe('getDeltaReport', () => {
idsToCreate: [],
idsToUpdate: [],
idsToDelete: [],
idsUnauthorized: []
})
})
it('Should return only one id to update as hashes are different', async () => {
Expand All @@ -171,6 +173,7 @@ describe('getDeltaReport', () => {
idsToCreate: [],
idsToUpdate: [bddCommonToponymMock[0].id],
idsToDelete: [],
idsUnauthorized: []
})
})
})
5 changes: 5 additions & 0 deletions lib/api/district/__mocks__/district-models.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@ import {bddDistrictMock} from './district-data-mock.js'
export async function getDistricts(districtIDs) {
return bddDistrictMock.filter(({id}) => districtIDs.includes(id))
}

export async function getCogFromDistrictID(districtID) {
const district = bddDistrictMock.find(({id}) => id === districtID)
return district ? district.meta?.insee?.cog : null
}
5 changes: 5 additions & 0 deletions lib/api/district/models.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ export const getDistrictsFromCog = cog => District.findAll({where: {meta: {insee

export const setDistricts = districts => District.bulkCreate(districts)

export const getCogFromDistrictID = async districtID => {
const district = await District.findByPk(districtID, {raw: true})
return district ? district.meta?.insee?.cog : null
}

export const updateDistricts = districts => {
const promises = districts.map(district => District.update({...district, isActive: true}, {where: {id: district.id}}))
return Promise.all(promises)
Expand Down
Loading