Save from repository admin detail page with no changes doesn't redirect to the listing page

[Ryan J Ollos]

Pressing Save when no changes have been made to a repository doesn't redirect back to the repository listing page. This appears to be a side-effect of the changes in [10042]. The other admin pages such as Ticket Components redirect back to the listing page when no changes have been made.

[Ryan J Ollos]

I debated several ways to fix this, including adding a if not changes: req.redirect(req.href.admin(category, page)) early in the function. Ultimately I felt that adding the valid variable was the cleanest solution, and may prove more useful if other validity checks are added. Let me know if the code doesn't seem clear, or there is a better way.

Proposed changes can be found in log:rjollos.git:t11355.

[Jun Omae]

LGTM, however I have one thing. value &= ...., it seems the trick. It would be simple to be the following.

trac/versioncontrol/admin.py

@@ -206 +206 @@
                         if (value is not None or field == 'hidden') \
                                 and value != info.get(field):
                             changes[field] = value
-                    if 'dir' in changes:
-                        valid &= self._check_dir(req, changes['dir'])
+                    if 'dir' in changes \
+                            and not self._check_dir(req, changes['dir']):
+                        valid = False
                     if valid and changes:
                         db_provider.modify_repository(reponame, changes)
                         add_notice(req, _('Your changes have been saved.'))

[Ryan J Ollos]

Committed with suggestion from comment:3 to 1.0-stable in [12231] and merged to trunk in [12232].

[Jun Omae]

In [12231], the valid variable never be set False. After the changes, if submitting non absolute path as repository directory in modify repository view, TracError would wrongly be shown. Before the changes, the modify repository view would be shown.

[Ryan J Ollos]

Thank you for catching my mistake. Fixed in [12233] and merged to trunk in [12234].

[Jun Omae]

I've found the following failure, fixed in [12251] and merged in [12252]. dir.lstrip('/') would be still absolute path on Windows.

======================================================================
FAIL: Test for regression of http://trac.edgewall.org/ticket/11355
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\versioncontrol\tests\functional.py", line 274, in runTest
    tc.url(self._tester.url + '/admin/versioncontrol/repository/' + name)
  File "C:\usr\src\trac\trac.git\trac\tests\functional\better_twill.py", line 236, in better_url
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ("current url is 'http://127.0.0.1:8032/admin/versioncontrol/repository';\ndoes not match 'http://127.0.0.1:8032/admin/versioncontrol/repository/EightOne'\n", 'C:\\usr\\src\\trac\\trac.git\\testenv\\trac\\log\\RegressionTestTicket11355.html')