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

feat(#648): add --disable-users command for merge-contacts action #649

Merged
merged 51 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
d16df6d
Testing this common library going to get weird
kennsippell Nov 7, 2024
3e1827e
Move-Contacts tests passing again
kennsippell Nov 7, 2024
d914e64
First test passing for merge
kennsippell Nov 12, 2024
090895c
Negative cases
kennsippell Nov 12, 2024
3e6168c
Fix move-contacts tests again
kennsippell Nov 12, 2024
2449dd4
Some renaming
kennsippell Nov 21, 2024
b5f8c3b
Refactor to use options
kennsippell Nov 21, 2024
1273fb6
Move folder structure
kennsippell Nov 21, 2024
25ad230
Lineage Constraints
kennsippell Nov 21, 2024
5ad9d85
Rename to Hierarchy Operations
kennsippell Nov 22, 2024
7ea3393
replaceRelevantLineage
kennsippell Nov 22, 2024
78f2c01
Refacatoring for lineage-manipulation
kennsippell Nov 23, 2024
d677b48
Tests for fn folder
kennsippell Nov 23, 2024
2442fcc
Pass eslint
kennsippell Nov 23, 2024
a0a0c84
Backend interface change
kennsippell Nov 23, 2024
f73f9c6
Fix failing test in mock-hierarchies
kennsippell Nov 23, 2024
8e35f2d
SonarCube
kennsippell Nov 23, 2024
17c4c04
SonarQube - Is his really better code?
kennsippell Nov 23, 2024
7af035c
SonarQube - Fix?
kennsippell Nov 23, 2024
687a6a2
SonarQube
kennsippell Nov 23, 2024
49c6d51
Oops
kennsippell Nov 23, 2024
9536ff6
Late night wireframe
kennsippell Nov 27, 2024
8b751b6
Passing with automation
kennsippell Nov 28, 2024
ad09272
After testing
kennsippell Nov 29, 2024
de78eb0
Passing eslint
kennsippell Nov 29, 2024
6d0cc3e
Reduced nesting via curried function
kennsippell Dec 6, 2024
e561431
4 feedbacks
kennsippell Dec 6, 2024
92ae094
Remove getHierarchyErrors public interface
kennsippell Dec 6, 2024
d68a294
Lots of lineage-constraints feedback
kennsippell Dec 6, 2024
c964aa7
Remove lineageAttribute
kennsippell Dec 6, 2024
88ea9fd
Still code reviewing
kennsippell Dec 6, 2024
296088a
Eslint
kennsippell Dec 6, 2024
42c6789
One more
kennsippell Dec 6, 2024
8f2bbd6
Why 5? wtf
kennsippell Dec 6, 2024
4ecf723
Phrasing
kennsippell Dec 6, 2024
8cb2840
Merge branch '373-merge-contacts-options' into 373-upload-docs-delete…
kennsippell Dec 6, 2024
0a9db49
Unneeded comment
kennsippell Dec 6, 2024
af9a9ac
lineage-manipulation refactor
kennsippell Dec 9, 2024
546f9cb
Docs
kennsippell Dec 9, 2024
fe27a5a
Oh that is why
kennsippell Dec 9, 2024
28be7fb
Remove function nesting
kennsippell Dec 9, 2024
99745c6
Last code review feedback
kennsippell Dec 9, 2024
6cb96d8
Merge branch '373-merge-contacts-options' into 373-upload-docs-delete…
kennsippell Dec 9, 2024
d4dcd45
No function nesting
kennsippell Dec 9, 2024
b323c9f
SonarCube after refactor
kennsippell Dec 9, 2024
bdbead6
Merge branch 'main' into 373-upload-docs-delete-user
kennsippell Dec 11, 2024
bd9bf60
Avoid breaking change with #652
kennsippell Dec 12, 2024
da6a8cb
Review feedback
kennsippell Dec 12, 2024
aebfb93
--disable-users also needed on upload-docs
kennsippell Dec 12, 2024
f729e80
Change tactics
kennsippell Dec 13, 2024
ec4effd
Docs update
kennsippell Dec 13, 2024
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
5 changes: 5 additions & 0 deletions src/fn/merge-contacts.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
const args = parseExtraArgs(environment.pathToProject, environment.extraArgs);
const db = pouch();
const options = {
disableUsers: args.disableUsers,
docDirectoryPath: args.docDirectoryPath,
force: args.force,
};
Expand Down Expand Up @@ -41,6 +42,7 @@ const parseExtraArgs = (projectDir, extraArgs = []) => {
return {
destinationId: args.destination,
sourceIds,
disableUsers: !!args['disable-users'],
docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'),
force: !!args.force,
};
Expand All @@ -63,6 +65,9 @@ ${bold('OPTIONS')}
--sources=<source_id1>,<source_id2>
A comma delimited list of IDs of contacts which will be deleted. The hierarchy of contacts and reports under it will be moved to be under the destination contact.

--disable-users
When flag is present, users at any deleted place will be updated and may be permanently disabled. Supported by CHT Core 4.7 and above.

--docDirectoryPath=<path to stage docs>
Specifies the folder used to store the documents representing the changes in hierarchy.
`);
Expand Down
122 changes: 104 additions & 18 deletions src/fn/upload-docs.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
const path = require('path');
const minimist = require('minimist');
const userPrompt = require('../lib/user-prompt');
const semver = require('semver');

const api = require('../lib/api');
const environment = require('../lib/environment');
const fs = require('../lib/sync-fs');
const { getValidApiVersion } = require('../lib/get-api-version');
const log = require('../lib/log');
const pouch = require('../lib/db');
const progressBar = require('../lib/progress-bar');
const userPrompt = require('../lib/user-prompt');

const { info, trace, warn } = log;

const FILE_EXTENSION = '.doc.json';
const INITIAL_BATCH_SIZE = 100;

const execute = async () => {
async function execute() {
const args = minimist(environment.extraArgs || [], { boolean: true });

const docDir = path.resolve(environment.pathToProject, args.docDirectoryPath || 'json_docs');
Expand All @@ -22,22 +25,23 @@ const execute = async () => {
return Promise.resolve();
}

const filesToUpload = fs.recurseFiles(docDir).filter(name => name.endsWith(FILE_EXTENSION));
const docIdErrors = getErrorsWhereDocIdDiffersFromFilename(filesToUpload);
if (docIdErrors.length > 0) {
throw new Error(`upload-docs: ${docIdErrors.join('\n')}`);
}

const totalCount = filesToUpload.length;
const filenamesToUpload = fs.recurseFiles(docDir).filter(name => name.endsWith(FILE_EXTENSION));
const totalCount = filenamesToUpload.length;
if (totalCount === 0) {
return; // nothing to upload
}

warn(`This operation will permanently write ${totalCount} docs. Are you sure you want to continue?`);
if (!userPrompt.keyInYN()) {
throw new Error('User aborted execution.');
const analysis = analyseFiles(filenamesToUpload);
const errors = analysis.map(result => result.error).filter(Boolean);
if (errors.length > 0) {
throw new Error(`upload-docs: ${errors.join('\n')}`);
}

userPrompt.warnPromptAbort(`This operation will permanently write ${totalCount} docs. Are you sure you want to continue?`);

const deletedDocIds = analysis.map(result => result.delete).filter(Boolean);
await handleUsersAtDeletedFacilities(deletedDocIds);

const results = { ok:[], failed:{} };
const progress = log.level > log.LEVEL_ERROR ? progressBar.init(totalCount, '{{n}}/{{N}} docs ', ' {{%}} {{m}}:{{s}}') : null;
const processNextBatch = async (docFiles, batchSize) => {
Expand Down Expand Up @@ -93,20 +97,102 @@ const execute = async () => {
}
};

return processNextBatch(filesToUpload, INITIAL_BATCH_SIZE);
};
return processNextBatch(filenamesToUpload, INITIAL_BATCH_SIZE);
}

const getErrorsWhereDocIdDiffersFromFilename = filePaths =>
filePaths
function analyseFiles(filePaths) {
return filePaths
.map(filePath => {
const json = fs.readJson(filePath);
const idFromFilename = path.basename(filePath, FILE_EXTENSION);

if (json._id !== idFromFilename) {
return `File '${filePath}' sets _id:'${json._id}' but the file's expected _id is '${idFromFilename}'.`;
return { error: `File '${filePath}' sets _id:'${json._id}' but the file's expected _id is '${idFromFilename}'.` };
}

if (json._deleted && json.cht_disable_linked_users) {
return { delete: json._id };
}
})
.filter(err => err);
.filter(Boolean);
}

async function handleUsersAtDeletedFacilities(deletedDocIds) {
if (!deletedDocIds?.length) {
return;
}

await assertCoreVersion();

const affectedUsers = await getAffectedUsers(deletedDocIds);
const usernames = affectedUsers.map(userDoc => userDoc.username).join(', ');
if (affectedUsers.length === 0) {
trace('No users found needing an update.');
return;
}

userPrompt.warnPromptAbort(`This operation will update ${affectedUsers.length} user accounts: ${usernames} and cannot be undone. Are you sure you want to continue?`);
await updateAffectedUsers(affectedUsers);
}

async function assertCoreVersion() {
const actualCoreVersion = await getValidApiVersion();
if (semver.lt(actualCoreVersion, '4.7.0-dev')) {
throw Error(`CHT Core Version 4.7.0 or newer is required to use --disable-users options. Version is ${actualCoreVersion}.`);
Copy link
Member

Choose a reason for hiding this comment

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

question: I've noticed an inconsistency in the command structure. The disable-users argument was moved from the disable-users command to the merge-contacts command. However, after reviewing the merge-contacts command, I don't see a direct invocation of upload-docs.

Are we looking at a scenario where these commands should be combined, such as cht merge-contacts upload-docs --disable-users?

My concern is that running this against a CHT 4.6.0 instance might result in an error, even if the --disable-users flag isn't explicitly passed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sugat009 You can read here about the intended use of the commands. Yes, you are right that the two need to be combined like this:

cht --local move-contacts upload-docs -- --contacts=contact_1 --parent=root

Copy link
Member Author

@kennsippell kennsippell Dec 12, 2024

Choose a reason for hiding this comment

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

My concern is that running this against a CHT 4.6.0 instance might result in an error, even if the --disable-users flag isn't explicitly passed.

I guess we have a few options:

  1. Crash against < 4.6.0 but only if deleted docs have the cht_disable_linked_users. Keep --disable-users flag on merge-contacts action and note that it is only supported by CHT Core 4.8 and above.
  2. Add --disable-users flag to upload-docs. Remove it from merge-contacts and (delete-contacts added in feat(#650): delete-contacts action #652 )
  3. Have --disable-users flag on both upload-docs and merge-contacts.

I suspect 1 is the best option?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, option 1 is the best option. Let's go with that.

Copy link
Member Author

@kennsippell kennsippell Dec 13, 2024

Choose a reason for hiding this comment

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

OK. Nice to confirm. That is what is in this PR now.

}

trace(`Core version is ${actualCoreVersion}. Proceeding to disable users.`);
}

async function getAffectedUsers(deletedDocIds) {
const toPostApiFormat = (apiResponse) => {
const places = Array.isArray(apiResponse.place) ? apiResponse.place.filter(Boolean) : [apiResponse.place];
const placeIds = places.map(place => place?._id);
return {
username: apiResponse.username,
place: placeIds,
};
};

const knownUserDocs = {};
for (const facilityId of deletedDocIds) {
const fetchedUserInfos = await api().getUsersAtPlace(facilityId);
for (const fetchedUserInfo of fetchedUserInfos) {
const userDoc = knownUserDocs[fetchedUserInfo.username] || toPostApiFormat(fetchedUserInfo);
removePlace(userDoc, facilityId);
knownUserDocs[userDoc.username] = userDoc;
}
}

return Object.values(knownUserDocs);
}

function removePlace(userDoc, placeId) {
if (Array.isArray(userDoc.place)) {
userDoc.place = userDoc.place
.filter(id => id !== placeId);
} else {
delete userDoc.place;
}
}

async function updateAffectedUsers(affectedUsers) {
let disabledUsers = 0, updatedUsers = 0;
for (const userDoc of affectedUsers) {
const shouldDisable = !userDoc.place || userDoc.place?.length === 0;
if (shouldDisable) {
trace(`Disabling ${userDoc.username}`);
await api().disableUser(userDoc.username);
disabledUsers++;
} else {
trace(`Updating ${userDoc.username}`);
await api().updateUser(userDoc);
updatedUsers++;
}
}

info(`${disabledUsers} users disabled. ${updatedUsers} users updated.`);
}

module.exports = {
requiresInstance: true,
Expand Down
24 changes: 24 additions & 0 deletions src/lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const request = {
get: _request('get'),
post: _request('post'),
put: _request('put'),
delete: _request('delete'),
};

const logDeprecatedTransitions = (settings) => {
Expand Down Expand Up @@ -98,6 +99,29 @@ const api = {
.then(() => updateAppSettings(content));
},

async getUsersAtPlace(facilityId) {
const result = await request.get({
uri: `${environment.instanceUrl}/api/v2/users?facility_id=${facilityId}`,
json: true,
});

return result || [];
},

disableUser(username) {
return request.delete({
uri: `${environment.instanceUrl}/api/v1/users/${username}`,
});
},

updateUser(userDoc) {
return request.post({
uri: `${environment.instanceUrl}/api/v1/users/${userDoc.username}`,
json: true,
body: userDoc,
});
},

createUser(userData) {
return request.post({
uri: `${environment.instanceUrl}/api/v1/users`,
Expand Down
1 change: 1 addition & 0 deletions src/lib/hierarchy-operations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ async function moveHierarchy(db, options, sourceIds, destinationId) {
_id: sourceDoc._id,
_rev: sourceDoc._rev,
_deleted: true,
cht_disable_linked_users: !!options.disableUsers,
});
}

Expand Down
11 changes: 10 additions & 1 deletion src/lib/user-prompt.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const environment = require('./environment');
const readline = require('readline-sync');
const { warn } = require('./log');


/**
Expand Down Expand Up @@ -33,8 +34,16 @@ function keyInSelect(items, question, options = {}) {
return readline.keyInSelect(items, question, options);
}

function warnPromptAbort(warningMessage) {
warn(warningMessage);
if (!keyInYN()) {
throw new Error('User aborted execution.');
}
}

module.exports = {
keyInYN,
question,
keyInSelect
keyInSelect,
warnPromptAbort,
};
1 change: 1 addition & 0 deletions test/fn/merge-contacts.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('merge-contacts', () => {
expect(parseExtraArgs(__dirname, args)).to.deep.eq({
sourceIds: ['food', 'is', 'tasty'],
destinationId: 'bar',
disableUsers: false,
force: true,
docDirectoryPath: '/',
});
Expand Down
Loading