Skip to content

Conversation

@GoranStefanovski
Copy link
Collaborator

Malce buchkurush jet ama ke se sredva poleka. Ne e finalno ova, samo kolku da raboti
Ke se prat relaci and shitoj uste, da ne povikvame balamurski apinja na sekoe.

Ama ubo e da se najdu kako Crud sho raboti.

Ete i so e aplikacijata
https://docs.google.com/document/d/1R-UHeO78mMEglwkzPCPQrsAQfhTELIxlbPXk06k7MZs/edit?usp=sharing

… for request, todo: addapt send mail to admin and manager
… and mail, confirmation page finish:FINISH BUTTONS
Copy link
Collaborator

Choose a reason for hiding this comment

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

add this file to gitignore

MAIL_PASSWORD=PASSWORD
MAIL_ENCRYPTION=tls
MAIL_FROM_ADDRESS=[email protected]
MAIL_FROM_NAME="Leave Management System"
Copy link
Collaborator

Choose a reason for hiding this comment

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

${APP_NAME} is more appropriate for this and not sure for the rest of the values

bool $is_disabled = false,
array $permissions_array = []
array $permissions_array = [],
int $paid_leaves_max,
Copy link
Collaborator

Choose a reason for hiding this comment

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

these attributes sound like are too specific to be included in the general user entity

const auth = useAuth();
const [block, element] = useBEMBuilder(
"admin-layout",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be leaved, will remove minimizing ablity

alt="Logo"
src="@/../assets/images/sm_logo_white.png"
/>
BreakPoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like business logic specific

Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like unnecessary around Datatable, unless should be extended in the future

requestToUser: UserFormItem;
}

export interface LeaveRequestsTableResponse {
Copy link
Collaborator

Choose a reason for hiding this comment

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

there should be generic type "PaginatedAPIResponse" or something to use here

@@ -0,0 +1,12 @@
import type { USER_PERMISSIONS, USER_ROLES } from "@/modules/users/constants";

export type Permission =
Copy link
Collaborator

Choose a reason for hiding this comment

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

use the types from users module, no need to redefine

</div>
</template>
<style scoped>
.color-picker {
Copy link
Collaborator

Choose a reason for hiding this comment

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

extract css

background: transparent;
cursor: pointer;
width: 40px;
height: 40px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

use css variables... prefer the ones defined in the core

manualLoading.value = false;
},
onError: (error) => {
// @ts-ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's not ignore TS

},
onError: (error) => {
// @ts-ignore
const firstErrorMessage = error.errors ? Object.values(error.errors)[0][0] : "An unexpected error occurred";
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe improve the format of the errors returned

@pashata
Copy link
Collaborator

pashata commented Feb 13, 2025

Better separate Frontend - Backend - Infrastructure changes in separate PR's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants