Edgewall Software
Modify

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#11355 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: admin/web Version: 1.0-stable
Severity: normal Keywords: repository
Cc: Branch:
Release Notes:

Pressing Save on Repository Admin edit page when no change is made redirects back to the listing page.

API Changes:
Internal Changes:

Description

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.

Attachments (0)

Change History (8)

comment:1 by Ryan J Ollos, 7 years ago

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.

comment:2 by Ryan J Ollos, 7 years ago

Status: newassigned

comment:3 by Jun Omae, 7 years ago

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

  • trac/versioncontrol/admin.py

    diff --git a/trac/versioncontrol/admin.py b/trac/versioncontrol/admin.py
    index 28d96c1..650e120 100644
    a b class RepositoryAdminPanel(Component):  
    206206                        if (value is not None or field == 'hidden') \
    207207                                and value != info.get(field):
    208208                            changes[field] = value
    209                     if 'dir' in changes:
    210                         valid &= self._check_dir(req, changes['dir'])
     209                    if 'dir' in changes \
     210                            and not self._check_dir(req, changes['dir']):
     211                        valid = False
    211212                    if valid and changes:
    212213                        db_provider.modify_repository(reponame, changes)
    213214                        add_notice(req, _('Your changes have been saved.'))

comment:4 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

comment:5 by Jun Omae, 7 years ago

Resolution: fixed
Status: closedreopened

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.

comment:6 by Ryan J Ollos, 7 years ago

Resolution: fixed
Status: reopenedclosed

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

comment:7 by Jun Omae, 7 years ago

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')

comment:8 by Ryan J Ollos, 6 years ago

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.