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

Add members #40

Open
wants to merge 12 commits into
base: development_2.0.0
Choose a base branch
from
Open

Add members #40

wants to merge 12 commits into from

Conversation

nabilasherif
Copy link

@nabilasherif nabilasherif commented Jul 31, 2024

Description
Add members page
Changes

  • modified components styles
  • connected with the backend

Related issues
#19

@nabilasherif nabilasherif marked this pull request as draft August 1, 2024 09:34
@nabilasherif nabilasherif marked this pull request as ready for review August 8, 2024 11:59
@nabilasherif nabilasherif changed the title intial template Add members Aug 8, 2024
async function Search(searchInput:any) {
await AuthClient.get(`/members/search/${searchInput}`)
.then(response=>{
console.log('Search results:', response.data);

Choose a reason for hiding this comment

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

remove console logs

Comment on lines 18 to 25
await AuthClient.get(`/members/search/${searchInput}`)
.then(response=>{
return response.data;
})
.catch(error=>{
console.error(error);
})
}

Choose a reason for hiding this comment

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

Suggested change
await AuthClient.get(`/members/search/${searchInput}`)
.then(response=>{
return response.data;
})
.catch(error=>{
console.error(error);
})
}
return await AuthClient.get(`/members/search/${searchInput}`)
}

and handle it with try and catch when you call it

<v-row >
<p class="mt-2 text-h4 text-blue-darken-4" variant="h5">ALL MEMBERS</p>
<v-spacer></v-spacer>
<v-btn color="primary" @click="addMemberDialog=true">

Choose a reason for hiding this comment

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

add loading prop

<v-card-actions>
<v-spacer></v-spacer>
<v-btn color="info" @click="addMemberDialog = false">Close</v-btn>
<v-btn color="success" @click="addMember">ADD+</v-btn>

Choose a reason for hiding this comment

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

Add loading and disabled prop

// take userinfo
};

const SearchMember = async () => {

Choose a reason for hiding this comment

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

implement notifier here

Copy link
Collaborator

@Mahmoud-Emad Mahmoud-Emad left a comment

Choose a reason for hiding this comment

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

Good job ya Nabila, some comments

required: true,
},
project_id: {
type: string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use String instead

baseURL: import.meta.env.VITE_APP_ENDPOINT,
baseURL: import.meta.env.VITE_APP_ENDPOINT,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Load it from the window instead

return await AuthClient.get(`/members/search/${searchInput}`)
}
async function getProjectMembers (projectId:any) {
return await AuthClient.get(`/members/project/${projectId}/members/`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

use members without projects, should be /api/members/all/
use this endpoint /members/project/${projectId}/members/ only inside an active project

Comment on lines +2 to +4
<div style="margin-left: 7cm; margin-right: 7cm;">
<v-container>
<v-row>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to load the navbar here

color="primary"
@click="addMemberDialog=true"
>
INVITE MEMBERS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
INVITE MEMBERS
Invite member

<div style="margin-left: 7cm; margin-right: 7cm;">
<v-container>
<v-row>
<p class="mt-2 text-h4 text-blue-darken-4" variant="h5">ALL MEMBERS</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p class="mt-2 text-h4 text-blue-darken-4" variant="h5">ALL MEMBERS</p>
<p class="mt-2 text-h4 text-blue-darken-4" variant="h5">All members</p>

Comment on lines 24 to 28
v-model="inviteNewMember.first_name"
density="compact"
placeholder="First Name"
prepend-inner-icon="mdi-account-outline"
variant="outlined"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add the validation rules, and apply it to all inputs

Comment on lines 44 to 48
<p>Permission</p>
<v-select
:items="['Full access', 'Admin access']"
label="Select"
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p>Permission</p>
<v-select
:items="['Full access', 'Admin access']"
label="Select"
/>
<v-select
:items="['Full access', 'Admin access']"
label="Permission"
/>

also, use an enum to normalizre the value, the backend require an exact pattern see the enum there

class PERMISSION_CHOICES(models.TextChoices):
    FULL_ACCESS = "full_access", "Full access"
    ADMIN_ACCESS = "admin_access", "Admin access"

so you need to send the full_access or admin_access as the value

Suggested change
<p>Permission</p>
<v-select
:items="['Full access', 'Admin access']"
label="Select"
/>
<v-select
:items="[{name: "Full access" value: 'full_access'}, {name: 'Admin access', value: 'admin_access'}]"
label="Permission"
/>

Comment on lines 116 to 120
const inviteNewMember = ref({
first_name: '',
last_name: '',
email: '',
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to define an interface for the data

Comment on lines 131 to 137
notifier.notify({
title: 'success',
description: 'member added successfully',
showProgressBar: true,
timeout: 7_000,
type: 'success',
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

The message will be returned from the backend

Comment on lines 4 to 13
baseURL: window.location.origin,
timeout: 1000,
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer ' + localStorage.getItem("token"),
Authorization: 'Bearer ' + localStorage.getItem('token'),
},
});
})

const BaseClient: AxiosInstance = axios.create({
baseURL: import.meta.env.VITE_APP_ENDPOINT,
baseURL: window.location.origin,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should use window.env.SERVER_DOMAIN_NAME_API instead


export { AuthClient, BaseClient };
async function search (searchInput:any) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we agreed we need to define the types of the parameters

@@ -39,7 +39,7 @@ class GetMemberApiView(GenericAPIView):
"""

serializer_class = MemberSerializers
permission_classes = (IsHost,)
# permission_classes = (IsHost,)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't commit the commented code

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