Skip to content

BAH-5159: Save privileges when importing a form #138

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.

Sign up for GitHub

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

Merged
kirsten-gilgenberg merged 12 commits into from
Oct 5, 2026
Merged

BAH-5159: Save privileges when importing a form #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3f17d6d
BAH-5159: Save privileges when importing a form
kirsten-gilgenberg Sep 29, 2026
bc3236a
BAH-5159: Fix privilege import when re-importing an existing form
kirsten-gilgenberg Sep 30, 2026
3d794ae
BAH-5159: Include privileges when exporting multiple forms
kirsten-gilgenberg Sep 30, 2026
c0ea8b2
BAH-5159: Refresh form list once after a bulk import finishes
kirsten-gilgenberg Sep 30, 2026
6cffa77
BAH-5150 | Fix. Change let to const for immutable variables
kirsten-gilgenberg Sep 30, 2026
4984f1d
BAH-5159: Fix broken privilege-save promise chain during form import
kirsten-gilgenberg Oct 1, 2026
ce5e81f
BAH-5159: Fix dropped return on the "form already exists" fallback GET
kirsten-gilgenberg Oct 1, 2026
bd61a99
BAH-5159: Extract shared buildFormPrivilegesPayload helper
kirsten-gilgenberg Oct 1, 2026
91b59e8
BAH-5159: Drop unused richer representation on form-create POST
kirsten-gilgenberg Oct 1, 2026
252838c
BAH-5159: Don't abort multi-form export over one privilege-fetch failure
kirsten-gilgenberg Oct 1, 2026
c62b9be
BAH-5159: Stop double-reporting form-resource save errors on import
kirsten-gilgenberg Oct 1, 2026
187efcd
BAH-5159: Fix CI failure in export success-notification test
kirsten-gilgenberg Oct 1, 2026
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
9 changes: 9 additions & 0 deletions src/common/apis/formPrivilegesApi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ import { UrlHelper } from 'form-builder/helpers/UrlHelper';
export function saveFormPrivileges(formPrivileges) {
return httpInterceptor.post(formBuilderConstants.saveFormPrivilegesUrl, formPrivileges);
}
export function buildFormPrivilegesPayload(formId, formVersion, formPrivileges) {
return formPrivileges.map((privilege) => ({
formId,
formVersion,
privilegeName: privilege.privilegeName,
editable: privilege.editable,
viewable: privilege.viewable,
}));
}
export function getFormPrivileges(formId, formVersion) {
return httpInterceptor.get(new UrlHelper()
.getFormPrivilegesUrl(formId, formVersion), 'text');
Expand Down
86 changes: 64 additions & 22 deletions src/form-builder/components/FormBuilder.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import { remove } from 'lodash';
import Spinner from 'common/Spinner';
import { formEventUpdate, saveEventUpdate } from 'form-builder/actions/control';
import { validateFormHyperlinks } from 'form-builder/helpers/hyperlinkValidationHelper';
import {
saveFormPrivileges, getFormPrivilegesFromUuid, buildFormPrivilegesPayload,
} from 'common/apis/formPrivilegesApi';

export default class FormBuilder extends Component {

Expand Down Expand Up @@ -62,7 +65,8 @@ export default class FormBuilder extends Component {
const version = this.getFormVersion(formName);
let uuid = '';
this.props.data.forEach(form => {
if (form.name === formName && form.version === version) {
// eslint-disable-next-line
if (form.name === formName && parseInt(form.version) === version) {
uuid = form.uuid;
}
});
Expand Down Expand Up @@ -238,6 +242,7 @@ export default class FormBuilder extends Component {
const formName = formJson.name;
const value = JSON.parse(formJson.resources[0].value);
const nameTranslations = formJson.resources[1] && formJson.resources[1].value;
const privileges = formJson.privileges || [];
const form = {
name: formName,
version: '1',
Expand All @@ -256,7 +261,9 @@ export default class FormBuilder extends Component {
});
self.updateImportErrors(fileName, message);
} else {
self.formJSONs.push({ form, value, formName, translations, nameTranslations });
self.formJSONs.push({
form, value, formName, translations, nameTranslations, privileges,
});
}
});
}
Expand Down Expand Up @@ -288,16 +295,28 @@ export default class FormBuilder extends Component {
const self = this;
const importFormJsonPromises = [];
formJsons.forEach(formJson => {
const { form, value, formName, translations, nameTranslations } = formJson;
const { form, value, formName, translations, nameTranslations, privileges } = formJson;
importFormJsonPromises.push(self.saveFormJson(form, value, formName, translations,
nameTranslations));
nameTranslations, privileges));
});
Promise.all(importFormJsonPromises)
.then(() => self.hideLoader())
.then(() => {
if (self.props.onImportComplete) {
self.props.onImportComplete();
}
self.hideLoader();
})
.catch(() => self.hideLoader());
}

saveFormJson(form, value, formName, translations, nameTranslations) {
saveImportedFormPrivileges(formId, formVersion, privileges) {

@hamsavarthiniR-Bahmni hamsavarthiniR-Bahmni Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

blocking: saveImportedFormPrivileges calls saveFormPrivileges(formPrivileges) without returning it (line 321), and both call sites invoke it from inside a .then((savedForm) => { ... }) body that itself returns undefined. So the privileges POST is fire-and-forget: a rejection becomes an unhandled promise rejection with no user feedback, and Promise.all(importFormJsonPromises) can resolve (hiding the loader, firing onImportComplete) before the privileges POST even finishes. For a fix whose whole purpose is "privileges silently dropped", this reopens the same failure mode.

return saveFormPrivileges(formPrivileges); here, and return self.saveImportedFormPrivileges(...) at both call sites, so failures propagate to the existing .catch(() => onValidationError(...)).

@kirsten-gilgenberg kirsten-gilgenberg Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 4984f1d: 4984f1d

saveImportedFormPrivileges now returns the saveFormPrivileges(...) promise, and both call sites return self.saveImportedFormPrivileges(...) so failures propagate to the existing .catch(() => onValidationError(...)).

if (!privileges || privileges.length === 0) {
return Promise.resolve();
}
return saveFormPrivileges(buildFormPrivilegesPayload(formId, formVersion, privileges));
}

saveFormJson(form, value, formName, translations, nameTranslations, privileges) {
const self = this;
const val = value;
const hyperlinkErrors = validateFormHyperlinks(val, this.props.allowedDomains || []);
Expand Down Expand Up @@ -327,16 +346,23 @@ export default class FormBuilder extends Component {
};
const translationsWithFormUuid = translations.map((eachTranslation) =>
Object.assign({}, eachTranslation, { formUuid: response.uuid }));
self.props.saveFormResource(formResource, translationsWithFormUuid,
formNameTranslationsResource);
return self.props.saveFormResource(formResource, translationsWithFormUuid,
formNameTranslationsResource)
.then((savedForm) =>
self.saveImportedFormPrivileges(savedForm.id, savedForm.version, privileges))
.catch(() => {
self.props.onValidationError(
`Import failed for form "${formName}": could not save form content`
);
});
})
.catch(() => {
const formUuid = self.getFormUuid(formName);
val.uuid = formUuid;
const params =
'v=custom:(id,uuid,name,version,published,auditInfo,' +
'resources:(value,dataType,uuid))';
httpInterceptor.get(`${formBuilderConstants.formUrl}/${formUuid}?${params}`)
return httpInterceptor.get(`${formBuilderConstants.formUrl}/${formUuid}?${params}`)
.then((data) => {
const formResource = {
form: {
Expand All @@ -354,7 +380,15 @@ export default class FormBuilder extends Component {
value: nameTranslations,
uuid: '',
};
self.props.saveFormResource(formResource, translations, formNameTranslationsResource);
return self.props
.saveFormResource(formResource, translations, formNameTranslationsResource)
.then((savedForm) =>
self.saveImportedFormPrivileges(savedForm.id, savedForm.version, privileges));
})
.catch(() => {
self.props.onValidationError(
`Import failed for form "${formName}": could not resolve the existing form`
);
});
});
}
Expand Down Expand Up @@ -436,7 +470,6 @@ export default class FormBuilder extends Component {
return;
}
const zip = new JSZip();
let fileName;
let params = '';
const uuids = this.state.selectedForms;
uuids.forEach((uuid, index) => {
Expand All @@ -450,19 +483,27 @@ export default class FormBuilder extends Component {
commonConstants.responseType.error);
}
const formData = exportResponse.bahmniFormDataList;
formData.forEach(form => {
fileName = `${form.formJson.name}_${form.formJson.version}`;
zip.file(`${fileName}.json`, JSON.stringify(form));
});
if (formData.length > 0) {
zip.generateAsync({ type: 'blob', compression: 'DEFLATE' }).then((content) => {
saveAs(content, commonConstants.exportFileName);
const privilegesPromises = formData.map((form) =>

@hamsavarthiniR-Bahmni hamsavarthiniR-Bahmni Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Promise.all(privilegesPromises) rejects as soon as any single form's getFormPrivilegesFromUuid call fails, aborting the entire multi-form export even though the other forms' data was already fetched successfully. Previously, export only depended on one endpoint succeeding.

Consider wrapping each getFormPrivilegesFromUuid(...) call in its own .catch(() => []) so one form's failure doesn't fail the whole batch.

@kirsten-gilgenberg kirsten-gilgenberg Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 252838c: 252838c

Wrapped each getFormPrivilegesFromUuid(...) call in .catch(() => []) so one form's privilege-fetch failure no longer aborts the whole batch export.

getFormPrivilegesFromUuid(form.formJson.uuid).catch(() => []));
return Promise.all(privilegesPromises).then((privilegesList) => {
formData.forEach((form, index) => {
const fileName = `${form.formJson.name}_${form.formJson.version}`;
const formWithPrivileges = Object.assign({}, form, {
formJson: Object.assign({}, form.formJson,
{ privileges: privilegesList[index] }),
});
zip.file(`${fileName}.json`, JSON.stringify(formWithPrivileges));
});
if (exportResponse.errorFormList.length === 0) {
this.setMessage(commonConstants.exportFormsSuccessMessage,
commonConstants.responseType.success);
if (formData.length > 0) {
zip.generateAsync({ type: 'blob', compression: 'DEFLATE' }).then((content) => {
saveAs(content, commonConstants.exportFileName);
});
if (exportResponse.errorFormList.length === 0) {
this.setMessage(commonConstants.exportFormsSuccessMessage,
commonConstants.responseType.success);
}
}
}
});
})
.catch(() => {
this.setMessage('Export failed', commonConstants.responseType.error);
Expand Down Expand Up @@ -529,6 +570,7 @@ FormBuilder.propTypes = {
isExact: PropTypes.bool.isRequired,
params: PropTypes.object,
}),
onImportComplete: PropTypes.func,
onValidationError: PropTypes.func,
routes: PropTypes.array,
saveForm: PropTypes.func.isRequired,
Expand Down
11 changes: 6 additions & 5 deletions src/form-builder/components/FormBuilderContainer.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@
}

getFormData() {
let initialForms = [];
let forms = [];
const initialForms = [];

Check warning on line 51 in src/form-builder/components/FormBuilderContainer.jsx

View workflow job for this annotation

GitHub Actions / Build & Package 

'initialForms' is assigned a value but never used
const forms = [];
const queryParams = '?=';

Check warning on line 53 in src/form-builder/components/FormBuilderContainer.jsx

View workflow job for this annotation

GitHub Actions / Build & Package 

'queryParams' is assigned a value but never used
const fetchFormsUrl = `${formBuilderConstants.formUrl}?v=custom:(id,uuid,name,version,published,auditInfo)`;

Check warning on line 54 in src/form-builder/components/FormBuilderContainer.jsx

View workflow job for this annotation

GitHub Actions / Build & Package 

Line 54 exceeds the maximum line length of 100
return httpInterceptor.get(fetchFormsUrl)
.then((initialForms) => {
this.collectAllForms(initialForms, forms);
Expand Down Expand Up @@ -156,7 +156,7 @@
saveFormResource(formJson, formTranslations, formNameTranslationsResource) {
const self = this;
self.setMessage('Importing Form...', commonConstants.responseType.success);
httpInterceptor.post(formBuilderConstants.bahmniFormResourceUrl, formJson)
return httpInterceptor.post(formBuilderConstants.bahmniFormResourceUrl, formJson)
.then((form) => {
const updatedTranslations = map(formTranslations, (translation) => {
const formTranslation = translation;
Expand All @@ -165,8 +165,8 @@
return formTranslation;
});
self.saveTranslations(updatedTranslations, formNameTranslationsResource);
})
.catch((error) => this.showErrors(error));
return form.form;
});
}

render() {
Expand All @@ -181,6 +181,7 @@
data={this.state.data}
dispatch={this.props.dispatch}
match={this.props.match}
onImportComplete={() => this.getFormData()}
onValidationError={(messages) => this.onValidationError(messages)}
routes={this.props.routes}
saveForm={(formName) => this.saveForm(formName)}
Expand Down
21 changes: 2 additions & 19 deletions src/form-builder/components/FormDetailContainer.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
} from 'common/apis/formTranslationApi';
import FormPreviewModal from 'form-builder/components/FormPreviewModal.jsx';
import Popup from 'reactjs-popup';
import { saveFormPrivileges } from 'common/apis/formPrivilegesApi';
import { saveFormPrivileges, buildFormPrivilegesPayload } from 'common/apis/formPrivilegesApi';
import { validateFormHyperlinks, fetchAllowedDomains } from 'form-builder/helpers/hyperlinkValidationHelper';

export class FormDetailContainer extends Component {
Expand Down Expand Up @@ -227,9 +227,8 @@ export class FormDetailContainer extends Component {
});
}
_saveFormPrivileges(formId, formVersion) {
let formVersionTemp = formVersion;
saveFormPrivileges(
this._createReqObject(formId, formVersionTemp, this.state.formPrivileges)
buildFormPrivilegesPayload(formId, formVersion, this.state.formPrivileges)
)
.then(() => {
const message = 'Form Privileges saved successfully';
Expand All @@ -242,22 +241,6 @@ export class FormDetailContainer extends Component {
});
}

_createReqObject(formId, formVersion, formPrivileges) {
const formPrivilegeObj = [];
for (let i = 0; i < formPrivileges.length; i++) {
const privilege = formPrivileges[i];
const privilegeCopy = {
formId,
privilegeName: privilege.privilegeName,
editable: privilege.editable,
viewable: privilege.viewable,
formVersion,
};
formPrivilegeObj.push(privilegeCopy);
}
return formPrivilegeObj;
}

onPublish() {
try {
const formJson = this.getFormResource();
Expand Down
Loading
Loading