Skip to content

Feature project delete view#89

Merged
kbeker merged 8 commits intomasterfrom
feature-project-delete-view
Apr 15, 2019
Merged

Feature project delete view#89
kbeker merged 8 commits intomasterfrom
feature-project-delete-view

Conversation

@MartynaAnnaGottschling
Copy link
Copy Markdown
Contributor

@MartynaAnnaGottschling MartynaAnnaGottschling commented Mar 8, 2019

Resolves #50
Resolves #51

self.project.save()
self.project.managers.add(self.user)
self.project.members.add(self.user)
self.url = reverse('custom-project-delete', args=(self.project.pk,))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that it would be better to keep this url as a string

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@MartynaAnnaGottschling MartynaAnnaGottschling force-pushed the feature-project-create-view branch from 5ae6623 to ee26823 Compare March 22, 2019 17:19
@MartynaAnnaGottschling MartynaAnnaGottschling force-pushed the feature-project-delete-view branch from a032878 to 55c35f3 Compare March 22, 2019 17:34
@MartynaAnnaGottschling MartynaAnnaGottschling force-pushed the feature-project-create-view branch 2 times, most recently from 07607ba to 4de302e Compare March 29, 2019 10:59
@MartynaAnnaGottschling MartynaAnnaGottschling force-pushed the feature-project-delete-view branch from 55c35f3 to 7049ae8 Compare March 29, 2019 11:00
@rwrzesien rwrzesien force-pushed the feature-project-create-view branch from 4de302e to 0a0cfbc Compare April 7, 2019 21:21
@rwrzesien rwrzesien removed their request for review April 7, 2019 22:47
@rwrzesien rwrzesien self-assigned this Apr 7, 2019
Comment thread managers/views.py Outdated


def delete_project(request, pk):
if request.user.user_type == CustomUser.UserType.ADMIN.name:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Karrp @MartynaAnnaGottschling Question about how this should exactly work - here implementation shows that only ADMIN should be allowed to perform this action. In template, everyone can click on this (which is a little bit miselading, I think it should be shown only for admins), however the real issue is that admins doesn't have access Projects List from the menu.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey, might be late to respond but unless you already applied some fixes, I can navigate to project list from menu, while signed in as admin.

And since all delete operations (except for individual reports) should be performed only by admin, you are right that the delete button should be visible only to admin.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But the delete button is in update template, which is accessable from detail template, only for admin, I think...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MartynaAnnaGottschling @maciejSamerdak I think I was able to reach it as manager too. Anyway, the point is that if it should be doable only by admin, only admin should be able to click on it - it might be bad user experience if there will be a button visible for users that when clicked will redirect them to home without telling them what actually happened. So we should agree on who can do and see what - for me both ways make sense (admin and manager who created this project or just admin), but whatever we choose lets be consistent.

We will probably solve more of the permissions cases in the future so lets just focus on this one now.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Karrp says only admin should be able to delete a project.

@rwrzesien rwrzesien force-pushed the feature-project-delete-view branch from 7049ae8 to 0ea2d58 Compare April 7, 2019 23:31
@kbeker kbeker added this to the tag 0.1.0 milestone Apr 8, 2019
@kbeker kbeker force-pushed the feature-project-create-view branch 2 times, most recently from 0a65109 to edfb40c Compare April 8, 2019 13:10
@kbeker kbeker changed the base branch from feature-project-create-view to master April 8, 2019 13:11
@rwrzesien rwrzesien force-pushed the feature-project-delete-view branch from 0ea2d58 to 9366ba5 Compare April 12, 2019 23:52
@rwrzesien rwrzesien force-pushed the feature-project-delete-view branch from 9366ba5 to a0cdd95 Compare April 12, 2019 23:55
Copy link
Copy Markdown
Contributor

@kbeker kbeker left a comment

Choose a reason for hiding this comment

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

OK for me :) merging

@kbeker kbeker merged commit a0cdd95 into master Apr 15, 2019
@kbeker kbeker deleted the feature-project-delete-view branch April 15, 2019 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create tests for projects views Manager Base Interface

4 participants