-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3254 - CAS AP Invoice - Invoices and Batches - UI #4305
#3254 - CAS AP Invoice - Invoices and Batches - UI #4305
Conversation
async getCASInvoiceBatches( | ||
paginationOptions: CASInvoiceBatchesPaginationOptions, | ||
): Promise<PaginatedResults<CASInvoiceBatch>> { | ||
const [results, count] = await this.casInvoiceBatchRepo.findAndCount({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Believe first time we are using paginated query in typeorm object query. 👍
skip: paginationOptions.pageLimit * paginationOptions.page, | ||
take: paginationOptions.pageLimit, | ||
order: { | ||
[paginationOptions.sortField]: paginationOptions.sortOrder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a default sort-field and order from backend perspective? or Are we expecting the Vue to handle the default sort field and order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the moment that we exposed the option to the API to receive the sort, I did not believe that required a default.
sortField?: string; | ||
@IsOptional() | ||
@IsArray() | ||
@Transform(({ value }) => value.split(",")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't follow the usage of this decorator here. I see it is used in programs search as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The query string will be provided as a regular comma-separated string in the URL, for instance, myQueryarameter=item1,item2,item3. This converts the comma-separated string to a nice array to be consumed by the controller. Besides converting it, the decorators will ensure the array items are also expected enum values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
sources/packages/web/src/composables/useInvoiceBatchApprovalStatus.ts
Outdated
Show resolved
Hide resolved
</template> | ||
<script lang="ts"> | ||
import { computed, defineComponent, PropType } from "vue"; | ||
import ChipStatus from "@/components/generic/ChipStatus.vue"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor. We can add this StatusChip
to main.ts as it is very generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to the main and changed existing files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much.
<full-page-container :full-width="true"> | ||
<template #header | ||
><header-navigator | ||
title="Corporate Accounting Services" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is initial version of UI, but should we call a page as Corporate Accounting Services
? Question to business.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Business was ok with the current naming at this moment, the preliminary UI was shared on different occasions.
We can wait for further feedback from the final users.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job. Thanks for moving the status chip to main and the refactor. 👍
* @returns list of all invoice batches. | ||
*/ | ||
@Get() | ||
@Roles(Role.AESTEditCASSupplierInfo) // TODO: Create a new role. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice work @andrewsignori-aot
New proposed UI to allow invoice batches download. The content is completely subject to be changed and the goal is to have a starting point to make some progress towards the final solution.
Note: the above UI was changed during PR to remove the "Only" prefix that can be noticed in the below images.