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

Fix error handling in fetchDocById helper #867

Merged
40 changes: 20 additions & 20 deletions src/components/auth/RegisterStudent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
name="noActivationCode"
@change="updateActivationCode"
/>
<label for="noActivationCode" class="ml-2">I don't have code</label>
<label for="noActivationCode" class="ml-2">I don't have a code</label>
</div>
</div>
<PvInputGroup v-if="!student.noActivationCode">
Expand Down Expand Up @@ -351,7 +351,6 @@ today.setFullYear(today.getFullYear() - 2);
const maxDoB = ref(today);
const orgName = ref('');
const activationCodeRef = ref('');
const errors = ref('');

const props = defineProps({
isRegistering: { type: Boolean, default: true },
Expand Down Expand Up @@ -529,31 +528,32 @@ const handleFormSubmit = async (isFormValid) => {
};

const validateCode = async (studentCode, outerIndex = 0) => {
if (studentCode && studentCode !== '') {
const activationCode = await fetchDocById('activationCodes', studentCode, undefined, 'admin', true, true).catch(
(error) => {
errors.value = error;
dialogMessage.value =
'The code does not belong to any organization \n Please enter a valid code or select: \n "I don`t have code "';
showErrorDialog();
submitted.value = false;
return null;
},
);
if (activationCode.orgId && errors.value === '') {
// @TODO: Add proper error handling.
if (!studentCode || studentCode === '') return;

try {
const activationCode = await fetchDocById('activationCodes', studentCode, undefined, 'admin', true, true);

if (activationCode.orgId) {
state.students[outerIndex].orgName = `${_capitalize(activationCode.orgType)} - ${
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we guarantee that these properties of activationCode i.e. activationCode.orgType exist? Should we add fallbacks or optional chaining?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't add optional chaining here so that if activationCode.orgType doesn't exist, an error is thrown and caught by the catch block. There is definitely big room for improvement on the error handling, but, in my opinion, that should be done in a separate PR?

activationCode.orgName ?? activationCode.orgId
}`;
state.students[outerIndex].activationCode = studentCode;
orgName.value = `${_capitalize(activationCode.orgType)} - ${activationCode.orgName ?? activationCode.orgId}`;
}
} catch (error) {
console.error('Failed to validate activation code', error);

if (!state.students[outerIndex].noActivationCode || props.code) {
dialogMessage.value = `The code ${studentCode} does not belong to any organization \n please enter a valid code or select: "I do not have a code"`;
} else {
errors.value = '';
if (!state.students[outerIndex].noActivationCode || props.code) {
dialogMessage.value = `The code ${studentCode} does not belong to any organization \n please enter a valid code or select: "I do not have code"`;
showErrorDialog();
}
return false;
dialogMessage.value =
'The code does not belong to any organization \n Please enter a valid code or select: \n "I don\'t have a code"';
}

showErrorDialog();

submitted.value = false;
}
};

Expand Down
28 changes: 9 additions & 19 deletions src/helpers/query/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ export const exportCsv = (data, filename) => {
* @param {Array<String>} [select] - Optional array of fields to select from the document.
* @param {String} [db=FIRESTORE_DATABASES.ADMIN] - The Firestore database to query.
* @param {Boolean} [unauthenticated=false] - Whether to use an unauthenticated request.
* @param {Boolean} [swallowErrors=false] - Whether to suppress error logging.
* @returns {Promise<Object>} The document data or an error message.
*/
export const fetchDocById = async (
Expand All @@ -137,7 +136,6 @@ export const fetchDocById = async (
select,
db = FIRESTORE_DATABASES.ADMIN,
unauthenticated = false,
swallowErrors = false,
) => {
const collectionValue = toValue(collection);
const docIdValue = toValue(docId);
Expand All @@ -148,27 +146,19 @@ export const fetchDocById = async (
);
return {};
}

const docPath = `/${collectionValue}/${docIdValue}`;
const axiosInstance = getAxiosInstance(db, unauthenticated);
const queryParams = (select ?? []).map((field) => `mask.fieldPaths=${field}`);
const queryString = queryParams.length > 0 ? `?${queryParams.join('&')}` : '';
return axiosInstance
.get(docPath + queryString)
.then(({ data }) => {
return {
id: docIdValue,
collectionValue,
..._mapValues(data.fields, (value) => convertValues(value)),
};
})
.catch((error) => {
if (!swallowErrors) {
console.error(error);
}
return {
data: `${error.code === '404' ? 'Document not found' : error.message}`,
};
});

const { data } = await axiosInstance.get(docPath + queryString);

return {
id: docIdValue,
collectionValue,
..._mapValues(data.fields, (value) => convertValues(value)),
};
};

/**
Expand Down