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

feat: add admin panel and display teams #175

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AurelienLoyer
Copy link
Collaborator

@AurelienLoyer AurelienLoyer commented Oct 30, 2020

GitHub issue/pull request detail

Description

Using the new admin menu
image
We are able now to manage teams and add a new team !

Screenshot

image

GIF !

@AurelienLoyer AurelienLoyer added Feature 🎉 New feature or request Work in progress 🚧 deploy-to-staging 🚜 Add this label to deploy your pull request to staging env hacktoberfest-accepted labels Oct 30, 2020
@AurelienLoyer
Copy link
Collaborator Author

Link to: #128

@AurelienLoyer
Copy link
Collaborator Author

@AurelienLoyer AurelienLoyer requested a review from ytvnr November 1, 2020 16:45
Copy link
Owner

@ytvnr ytvnr left a comment

Choose a reason for hiding this comment

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

some remarks and questions

Comment on lines +12 to +13
path: 'organisations',
name: 'organisations',
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
path: 'organisations',
name: 'organisations',
path: 'organizations',
name: 'organizations',

{
path: 'users',
name: 'users',
component: Dashboard,
Copy link
Owner

Choose a reason for hiding this comment

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

component Dashboard for user ? typo or waiting for a future component ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For future admin and god user !

return {
links: [
{ link: '', label: 'Dashboard' },
{ link: 'organisations', label: 'Organisations', isDisabled: true },
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
{ link: 'organisations', label: 'Organisations', isDisabled: true },
{ link: 'organizations', label: 'Organisations', isDisabled: true },

Comment on lines +19 to +22
{ link: '', label: 'Dashboard' },
{ link: 'organisations', label: 'Organisations', isDisabled: true },
{ link: 'teams', label: 'Teams', isDisabled: false },
{ link: 'users', label: 'Users', isDisabled: true },
Copy link
Owner

Choose a reason for hiding this comment

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

do you think it is posible to reuse the array from router/admin.js to avoid rewriting this array ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure and it will be different array for admin and god

Comment on lines +26 to +30
components: {
},
created(){

}
Copy link
Owner

Choose a reason for hiding this comment

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

can we delete them ?

methods: {

async getTeams() {
const teams = await this.concatDocument(firebase.firestore().collection('teams').get());
Copy link
Owner

Choose a reason for hiding this comment

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

can we use the teams.service ?

this.teams = teams;
},
async getOrganizations() {
this.organizations = await this.concatDocument(firebase.firestore().collection('organizations').get());
Copy link
Owner

Choose a reason for hiding this comment

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

organization service ?

this.dialog = false
this.$nextTick(() => {
this.editedItem = Object.assign({}, this.defaultItem)
this.editedId = null;
Copy link
Owner

Choose a reason for hiding this comment

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

do we need editedId field if we already have editedItem (which hold the id too)?

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 reuse the vuetify way but yes this.editedItem.id should exist

save () {
const orgaId = this.editedItem.organization.id || this.editedItem.organization;
if (this.editedId) {
delete this.editedItem.id;
Copy link
Owner

Choose a reason for hiding this comment

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

why 'delete' and not this.editedItem.id = null ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its for prevent to send the id to firebase

const orgaId = this.editedItem.organization.id || this.editedItem.organization;
if (this.editedId) {
delete this.editedItem.id;
firebase.firestore().collection('teams')
Copy link
Owner

Choose a reason for hiding this comment

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

can we use the teams service ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-to-staging 🚜 Add this label to deploy your pull request to staging env Feature 🎉 New feature or request hacktoberfest-accepted Work in progress 🚧
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants