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

[admin] delete user from grid #3151

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions admin/acceptance_tests/user_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest
from pyramid.testing import DummyRequest
from selenium.common.exceptions import NoSuchElementException


@pytest.fixture(scope='class')
Expand Down Expand Up @@ -48,6 +49,21 @@ def test_view_edit(self, dbsession, test_app):
('secretary_3', False, 'secretary_3')]
assert resp.form['role_name'].value == user.role_name

def test_delete_success(self, dbsession):
from c2cgeoportal_commons.models.main import User
user_id = dbsession.query(User). \
filter(User.username == 'babar_9'). \
one().id
from c2cgeoportal_admin.views.users import UserViews
req = DummyRequest(dbsession=dbsession)
req.referer = 'this is a test, TestUser-test_delete'
req.matchdict.update({'id': str(user_id)})

UserViews(req).delete()
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer we pass through the entire app using test_app


user = dbsession.query(User).filter(User.id == user_id).one_or_none()
assert user is None

Copy link
Member

Choose a reason for hiding this comment

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

For me test of return status and location header is missing here

@pytest.mark.usefixtures("test_app") # route have to be registred for HTTP_FOUND
def test_submit_update(self, dbsession, test_app):
from c2cgeoportal_commons.models.main import User
Expand Down Expand Up @@ -150,3 +166,39 @@ def test_selenium(self, dbsession, selenium):
dbsession.expire(user)
assert user.username == 'new_name_éôù'
assert user.email == 'new_email'

# in order to make this work, had to install selenium gecko driver
@pytest.mark.usefixtures("selenium", "selenium_app")
def test_delete_selenium(self, dbsession, selenium):
from c2cgeoportal_commons.models.main import User
user_id = dbsession.query(User). \
filter(User.username == 'babar_13'). \
one().id
selenium.get('http://127.0.0.1:6543' + '/user/')
elem = selenium.find_element_by_xpath("//div[@class='infos']")
assert 'Showing 1 to 10 of 23 entries' == elem.text
Copy link
Member

Choose a reason for hiding this comment

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

is this related with delete ?

Copy link
Contributor Author

@cmangeat cmangeat Oct 18, 2017

Choose a reason for hiding this comment

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

Ensuring that there 23 entries before delete, then 22 after delete, also that all entries can be read on the same page (50 per pages, i.e. more than 22) and that the good one disappear.

main_window_handle = selenium.current_window_handle
Copy link
Member

Choose a reason for hiding this comment

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

main_window_handle seems unused

elem = selenium.find_element_by_xpath("//button[@title='Refresh']/following-sibling::*")
elem.click()
elem = selenium.find_element_by_xpath("//a[contains(.,'50')]")
elem.click()
elem = selenium.find_element_by_xpath("//a[contains(@href,'" +
str(user_id) +
"/edit')]/following-sibling::*")
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that delete will stay the edit following-sibling, this is hard to understand, is it not possible to get the delete button directly ?

Copy link
Contributor Author

@cmangeat cmangeat Oct 18, 2017

Choose a reason for hiding this comment

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

Need to add a ref for this in template at c2cgeoform template side, no sure that we want to (for delete, id is lost somewhere in a js var)

Copy link
Member

Choose a reason for hiding this comment

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

We will have tu put a class on it for the icon, this could be addressed in this PR too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Icon don't carry id ! e.g. user.id.

elem.click()
selenium.switch_to_alert().accept()

selenium.switch_to_default_content()
selenium.refresh()
elem = selenium.find_element_by_xpath("//div[@class='infos']")
assert 'Showing 1 to 10 of 22 entries' == elem.text
elem = selenium.find_element_by_xpath("//button[@title='Refresh']/following-sibling::*")
elem.click()
Copy link
Member

Choose a reason for hiding this comment

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

Is it stricly neccessary to do such many refreshes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems that gecko driver need a little time to shift focus exiting alert window. I Prefer calling refresh than adding tempo.

elem = selenium.find_element_by_xpath("//a[contains(.,'50')]")
elem.click()
try:
assert None == selenium.find_element_by_xpath("//a[contains(@href,'" +
str(user_id) +
"/edit')]/following-sibling::*")
except NoSuchElementException:
pass
4 changes: 4 additions & 0 deletions admin/c2cgeoportal_admin/views/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ def view(self):
renderer="../templates/edit.jinja2")
def save(self):
return super().save()

@view_config(route_name='c2cgeoform_delete')
Copy link
Member

Choose a reason for hiding this comment

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

Should be updated accordingly to routes in c2cgeoform after merge of camptocamp/c2cgeoform#117

def delete(self):
return super().delete()
4 changes: 4 additions & 0 deletions admin/c2cgeoportal_admin/views/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ def view(self):
renderer="../templates/edit.jinja2")
def save(self):
return super().save()

@view_config(route_name='c2cgeoform_delete')
def delete(self):
return super().delete()
2 changes: 1 addition & 1 deletion admin/requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
-e git://github.com/camptocamp/c2cgeoform.git@9718b29415707d08fecfdced62cb165a64826f2e#egg=c2cgeoform
-e git://github.com/camptocamp/c2cgeoform.git@123561e963e43b765040e973c92599159cbd2170#egg=c2cgeoform
Copy link
Member

Choose a reason for hiding this comment

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

need to be updated after camptocamp/c2cgeoform#117 is merged

2 changes: 1 addition & 1 deletion commons/requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
-e git://github.com/camptocamp/c2cgeoform.git@87a4191ef3330b76497bd009e9c0220cbc73c625#egg=c2cgeoform
-e git://github.com/camptocamp/c2cgeoform.git@123561e963e43b765040e973c92599159cbd2170#egg=c2cgeoform