Apache Allura™

Forge software for hosting software projects

Brought to you by: alexluberg, brondsem, ccruz, deshani, and 11 others

Git Merge Request #293: [#8225] Redirect to root of project if object is deleted (merged)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Shalitha Suranga wants to merge 4 commits from /u/shalithasuranga/allura/ to master, 2018-10-04

Commit	Date
[52357c] (redirectfix) by Shalitha [#8225] Fix skip 404 by validatiing request.referer	2018-10-04 08:04:49	Tree
[243bbe] by Shalitha [#8225] Skip 404 for non root and only when deleted	2018-10-03 09:00:19	Tree
[38666c] by Shalitha Suranga Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/allura into redirectfix	2018-10-02 09:44:16	Tree
[a25e33] by Shalitha Suranga [#8225] Redirect if current object is deleted	2018-10-02 09:43:49	Tree

Discussion

Shalitha Suranga - 2018-10-02

This fix skips 404 error when user at mount point exactly means that when user clicks "Delete Everything" on module which is going to be deleted.. when user delete from http://localhost:8080/p/allura/blog2/new it will display 404 but not delete from http://localhost:8080/p/allura/blog2/

Do we need to skip 404 from urls like http://localhost:8080/p/allura/blog2/new too?

Last edit: Shalitha Suranga 2018-10-02

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2018-10-02

Yes I think it would be good to avoid the 404 page for any path within a tool that is deleted.

A few tests in the Allura and ForgeLink packages fail with this change. It looks like those tests and other code paths use update_mounts to install tools too, so that should be kept in mind - seems to function fine when I install a tool though.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Shalitha Suranga - 2018-10-03

Hi.. Dave

Added fix to redirect root only when delete and regex mached with probject/mountpoint also
Btw. Can you advice how you run unit tests for selected module only ? Are you using python -m unittest discover ?

Thanks

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2018-10-04

https://forge-allura.apache.org/docs/development/contributing.html#testing has some notes about testing. It doesn't mention it but you can also run tests for a single file like: nosetests path/to/file.py or for a single class or method like: nosetests some.package:TestClass.test_foo

The allura.tests.functional.test_admin:TestProjectAdmin.test_admin_controller test fails for me still. I think its because referrer can be None in tests. We do request.referer or ' in some places to avoid problems like this one.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Shalitha Suranga - 2018-10-04

Hi.. Dave

Thanks nosetests is good option for running only selected case. I did some fix and now no issues with above test. For all future fixes I will run nosetests before committing

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2018-10-04

Status: open --> merged
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Log in to post a comment.