Facilities refactored

[Banibrata-Manna]

Related Issues

Short Description and Why It's Useful

Screenshots of Visual Changes before/after (If There Are Any)

Contribution and Currently Important Rules Acceptance

• I read and followed contribution rules

[gemini-code-assist]

Code Review

This pull request migrates the application from Vue CLI and Vuex to Vite and Pinia, refactoring components to the <script setup> syntax and updating environment variable and API call structures. The code review identified several critical and high-severity issues across the changes. Notably, a runtime TypeError was found in Image.vue due to component name shadowing of the global Image constructor, and a missing baseURL in the createFacility store action. Other issues include insufficient error checking on Promise.allSettled results, potential input validation lag, missing error notifications for phone number updates, a potential runtime crash in Settings.vue due to missing optional chaining on userProfile, and a lack of validation for calendar selection before saving operating hours.

[gemini-code-assist]

In Vue 3 <script setup>, the component name is inferred from the filename (Image.vue), which shadows the global Image constructor. Calling new Image() will attempt to instantiate the Vue component as a constructor, throwing a TypeError: Image is not a constructor at runtime. Use new window.Image() or document.createElement('img') to reference the global HTMLImageElement constructor.

function checkIfImageExists(src: string) {
  return new Promise((resolve, reject) => {
    const img = new window.Image();
    img.onload = () => resolve(true);
    img.onerror = () => reject(false);
    img.src = src;
  });
}

[gemini-code-assist]

The api call is missing the baseURL parameter. Since other actions in this store explicitly define baseURL: commonUtil.getOmsURL(), omitting it here will cause these requests to target the wrong endpoint or fail entirely if the default baseURL is different. Please ensure all api calls in this store consistently specify the baseURL.

    async createFacility(payload: any) {
      return api({
        baseURL: commonUtil.getOmsURL(),
        url: "admin/facilities",
        method: "post",
        data: payload
      })
    },

[gemini-code-assist]

Checking only response.status === 'rejected' is insufficient when using Promise.allSettled because API calls that return a 200 OK with an error payload (common in OFBiz/HotWax OMS) will resolve successfully and be marked as 'fulfilled'. To correctly identify failures, you must also check commonUtil.hasError(response.value).

  const responses = await Promise.allSettled([...removePromises, ...addPromises]);
  const hasFailed = responses.some((response: any) => 
    response.status === 'rejected' || 
    (response.status === 'fulfilled' && commonUtil.hasError(response.value))
  );

[gemini-code-assist]

Validating input length using the potentially stale formData property instead of the immediate value from the event can lead to validation lag. Since @ionInput fires before v-model might have fully propagated the update to the reactive state, using value.length is much safer and more immediate.

function validateFacilityGroupId(event: any) {
  const value = event.target.value;
  if (!facilityGroupIdInput.value) return;

  const el = facilityGroupIdInput.value.$el;
  el.classList.remove('ion-valid');
  el.classList.remove('ion-invalid');

  if (value === '') return;

  value.length <= 20
    ? el.classList.add('ion-valid')
    : el.classList.add('ion-invalid');
}

[gemini-code-assist]

If updating the phone number fails, the error is caught and logged, but no toast notification is shown to the user. The modal dismisses and shows a success message for the address update, leading to a misleading user experience where some updates silently failed. Please show a toast notification on failure.

async function saveTelecomNumber() {
  let resp = {} as any;
  const payload = { 
    facilityId: props.facilityId,
    contactMechPurposeTypeId: 'PRIMARY_PHONE',
    contactNumber: telecomNumberValue.value.contactNumber.trim(),
    countryCode: telecomNumberValue.value.countryCode.replace('+', '')
  };

  try {
    if (contactDetails.value.telecomNumber?.contactMechId) {
      resp = await facilityStore.updateFacilityTelecomNumber({
        ...payload,
        contactMechId: contactDetails.value.telecomNumber.contactMechId,
      });
    } else {
      resp = await useFacilityStore().createFacilityTelecomNumber(payload);
    }

    if (!commonUtil.hasError(resp)) {
      await facilityStore.fetchFacilityContactDetailsAndTelecom({ facilityId: props.facilityId });
    } else {
      throw resp.data;
    }
  } catch (err) {
    commonUtil.showToast(translate("Failed to update phone number."));
    logger.error(err);
  }
}

[gemini-code-assist]

userProfile is initialized as {} and can be empty or null. Accessing userProfile.username directly without optional chaining can cause a runtime crash, whereas userProfile?.userFullName is correctly guarded. Please use optional chaining for username as well.

            <ion-card-header class="ion-no-padding ion-padding-vertical">
              <ion-card-subtitle>{{ userProfile?.username }}</ion-card-subtitle>
              <ion-card-title>{{ userProfile?.userFullName }}</ion-card-title>
            </ion-card-header>

[gemini-code-assist]

If the user clicks "Save" without selecting any calendar, selectedCalendarId.value will be empty/undefined, leading to an invalid API call. Please add a validation check to ensure a calendar is selected before proceeding.

async function addOperatingHours() {
  if (!selectedCalendarId.value) {
    commonUtil.showToast(translate("Please select a calendar."));
    return;
  }
  emitter.emit('presentLoader');

  try {
    const resp = await facilityStore.associateCalendarToFacility({
      facilityId: props.facilityId,
      calendarId: selectedCalendarId.value,
      fromDate: DateTime.now().toMillis(),
      facilityCalendarTypeId: 'OPERATING_HOURS'
    });

[patelanil]

Log message format review

I reviewed the log/error messages added in this PR against our best practice: log-messages.md.

These added messages do not follow the [Entity] [Context] - [Action/Outcome/Issue] pattern:

1. Failed to add group to facility
   Suggest: Facility - Failed to add group
2. Failed to remove group from facility
   Suggest: Facility - Failed to remove group
3. Failed to find facility groups
   Suggest: Facility - Failed to find facility groups
4. Failed to create facility location
   Suggest: Facility - Failed to create location
5. Failed to update facility location
   Suggest: Facility - Failed to update location
6. Failed to create external mapping
   Suggest: Facility - Failed to create external mapping
7. Unable to find the latitude and longitude for the entered zip code.
   Suggest: Facility - Unable to find latitude and longitude for the entered zip code
8. Failed to remove party from facility
   Suggest: Facility - Failed to remove party

...and 36 more in the same pattern.

Dynamic IDs belong in the [Context] block and the action text should stay constant, so messages group and count cleanly in Grafana/Loki during log analysis. Please align these before merge.

[ymaheshwari1]

Closing as this has been moved to company app and thus not required.