[admin] delete user from grid #3151

cmangeat · 2017-10-06T13:58:21Z

linked to camptocamp/c2cgeoform#117
camptocamp/c2cgeoform@17def80

arnaud-morvan · 2017-10-16T07:52:56Z

@sbrunner: Why do you closed this PR ? For me it should be rebased on master.

sbrunner · 2017-10-16T07:56:35Z

I delete the branch use_node_modules_jinja2, and probably GitHub automatically close this pull request ...

arnaud-morvan · 2017-10-18T11:35:57Z

admin/requirements.txt

@@ -1 +1 @@
-e git://github.com/camptocamp/c2cgeoform.git@9718b29415707d08fecfdced62cb165a64826f2e#egg=c2cgeoform
+-e git://github.com/camptocamp/c2cgeoform.git@123561e963e43b765040e973c92599159cbd2170#egg=c2cgeoform


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

arnaud-morvan

On a general side, I would prefer test only the things related to delete in delete, here are duplicates of other tests, it increase the complexity and difficulty to understand the test code.

arnaud-morvan · 2017-10-18T11:50:01Z

admin/c2cgeoportal_admin/views/roles.py

@@ -35,3 +35,7 @@ def view(self):
                 renderer="../templates/edit.jinja2")
    def save(self):
        return super().save()
+
+    @view_config(route_name='c2cgeoform_delete')


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

arnaud-morvan · 2017-10-18T11:51:54Z

admin/acceptance_tests/user_test.py

+        req.referer = 'this is a test, TestUser-test_delete'
+        req.matchdict.update({'id': str(user_id)})
+
+        UserViews(req).delete()


I would prefer we pass through the entire app using test_app

arnaud-morvan · 2017-10-18T11:52:42Z

admin/acceptance_tests/user_test.py

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


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

arnaud-morvan · 2017-10-18T11:53:45Z

admin/acceptance_tests/user_test.py

+        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
+        main_window_handle = selenium.current_window_handle


main_window_handle seems unused

arnaud-morvan · 2017-10-18T11:54:04Z

admin/acceptance_tests/user_test.py

+            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


is this related with delete ?

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.

arnaud-morvan · 2017-10-18T11:56:20Z

admin/acceptance_tests/user_test.py

+        elem.click()
+        elem = selenium.find_element_by_xpath("//a[contains(@href,'" +
+                                              str(user_id) +
+                                              "/edit')]/following-sibling::*")


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 ?

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)

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

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

arnaud-morvan · 2017-10-18T11:57:07Z

admin/acceptance_tests/user_test.py

+        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()


Is it stricly neccessary to do such many refreshes

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

arnaud-morvan · 2017-10-27T11:49:35Z

Superseeded by #3208

sbrunner approved these changes Oct 6, 2017

View reviewed changes

cmangeat changed the base branch from commons_cleaning_pr_cmt_bak to master October 9, 2017 06:38

cmangeat force-pushed the admin_delete branch 2 times, most recently from b64bb82 to d84c6e1 Compare October 12, 2017 16:18

cmangeat changed the base branch from master to use_node_modules_jinja2 October 12, 2017 16:27

cmangeat force-pushed the admin_delete branch 2 times, most recently from 1de06d1 to 1d5ca40 Compare October 13, 2017 08:39

arnaud-morvan force-pushed the use_node_modules_jinja2 branch 2 times, most recently from 3ab8ff4 to 588d640 Compare October 13, 2017 11:27

cmangeat requested a review from arnaud-morvan October 13, 2017 11:27

sbrunner closed this Oct 16, 2017

sbrunner deleted the admin_delete branch October 16, 2017 06:55

sbrunner changed the base branch from use_node_modules_jinja2 to master October 16, 2017 07:56

sbrunner restored the admin_delete branch October 16, 2017 07:56

sbrunner reopened this Oct 16, 2017

cmangeat added 3 commits October 18, 2017 13:32

[admin] delete user from grid

b3f0621

[admin] test for user delete with confirm popup

587ff47

target the 'right' geoform

1555634

arnaud-morvan force-pushed the admin_delete branch from 1d5ca40 to 1555634 Compare October 18, 2017 11:34

arnaud-morvan reviewed Oct 18, 2017

View reviewed changes

cmangeat force-pushed the admin_delete branch from 1555634 to d6f10a3 Compare October 18, 2017 12:58

arnaud-morvan force-pushed the admin_delete branch from 877d388 to 1555634 Compare October 19, 2017 16:46

arnaud-morvan mentioned this pull request Oct 27, 2017

[admin] Add delete link in grid #3208

Merged

arnaud-morvan closed this Oct 27, 2017

sbrunner deleted the admin_delete branch October 27, 2017 12:21

sbrunner added this to the dummy milestone Apr 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[admin] delete user from grid #3151

[admin] delete user from grid #3151

cmangeat commented Oct 6, 2017 •

edited

arnaud-morvan commented Oct 16, 2017

sbrunner commented Oct 16, 2017

arnaud-morvan Oct 18, 2017

arnaud-morvan left a comment

arnaud-morvan Oct 18, 2017

arnaud-morvan Oct 18, 2017

arnaud-morvan Oct 18, 2017

arnaud-morvan Oct 18, 2017

arnaud-morvan Oct 18, 2017

cmangeat Oct 18, 2017 •

edited

arnaud-morvan Oct 18, 2017

cmangeat Oct 18, 2017 •

edited

arnaud-morvan Oct 19, 2017

cmangeat Oct 19, 2017

arnaud-morvan Oct 18, 2017

cmangeat Oct 18, 2017

arnaud-morvan commented Oct 27, 2017

		@@ -1 +1 @@
		-e git://github.com/camptocamp/c2cgeoform.git@9718b29415707d08fecfdced62cb165a64826f2e#egg=c2cgeoform
		-e git://github.com/camptocamp/c2cgeoform.git@123561e963e43b765040e973c92599159cbd2170#egg=c2cgeoform


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

[admin] delete user from grid #3151

[admin] delete user from grid #3151

Conversation

cmangeat commented Oct 6, 2017 • edited

arnaud-morvan commented Oct 16, 2017

sbrunner commented Oct 16, 2017

Choose a reason for hiding this comment

arnaud-morvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmangeat Oct 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmangeat Oct 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnaud-morvan commented Oct 27, 2017

cmangeat commented Oct 6, 2017 •

edited

cmangeat Oct 18, 2017 •

edited

cmangeat Oct 18, 2017 •

edited