Mercurial > kallithea
changeset 4363:7ad20e8f16bd
redirect: don't redirect on error - it is better to have clear error messages and stable and meaningful URLs
| author | Mads Kiilerich <madski@unity3d.com> |
|---|---|
| date | Fri, 18 Jul 2014 19:22:01 +0200 |
| parents | 79eb3211cda1 |
| children | e50e6384c529 |
| files | kallithea/controllers/changelog.py kallithea/controllers/compare.py kallithea/controllers/files.py kallithea/lib/base.py |
| diffstat | 4 files changed, 41 insertions(+), 61 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/changelog.py Fri Jul 18 19:21:56 2014 +0200 +++ b/kallithea/controllers/changelog.py Fri Jul 18 19:22:01 2014 +0200 @@ -71,12 +71,10 @@ super(ChangelogController, self).__before__() c.affected_files_cut_off = 60 - def __get_cs_or_redirect(self, rev, repo, redirect_after=True, - partial=False): + @staticmethod + def __get_cs(rev, repo): """ - Safe way to get changeset if error occur it redirects to changeset with - proper message. If partial is set then don't do redirect raise Exception - instead + Safe way to get changeset. If error occur fail with error message. :param rev: revision to fetch :param repo: repo instance @@ -85,18 +83,12 @@ try: return c.db_repo_scm_instance.get_changeset(rev) except EmptyRepositoryError, e: - if not redirect_after: - return None h.flash(h.literal(_('There are no changesets yet')), - category='warning') - redirect(url('changelog_home', repo_name=repo.repo_name)) - + category='error') except RepositoryError, e: log.error(traceback.format_exc()) - h.flash(safe_str(e), category='warning') - if not partial: - redirect(h.url('changelog_home', repo_name=repo.repo_name)) - raise HTTPBadRequest() + h.flash(safe_str(e), category='error') + raise HTTPBadRequest() @LoginRequired() @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', @@ -136,7 +128,7 @@ except (NodeDoesNotExistError, ChangesetError): #this node is not present at tip ! try: - cs = self.__get_cs_or_redirect(revision, repo_name) + cs = self.__get_cs(revision, repo_name) collection = cs.get_file_history(f_path) except RepositoryError, e: h.flash(safe_str(e), category='warning')
--- a/kallithea/controllers/compare.py Fri Jul 18 19:21:56 2014 +0200 +++ b/kallithea/controllers/compare.py Fri Jul 18 19:22:01 2014 +0200 @@ -58,17 +58,10 @@ def __before__(self): super(CompareController, self).__before__() - def __get_rev_or_redirect(self, ref, repo, redirect_after=True, - partial=False): + @staticmethod + def __get_rev(ref, repo): """ - Safe way to get changeset if error occur it redirects to changeset with - proper message. If partial is set then don't do redirect raise Exception - instead - - :param ref: - :param repo: - :param redirect_after: - :param partial: + Safe way to get changeset. If error occurs show error. """ rev = ref[1] # default and used for git if repo.scm_instance.alias == 'hg': @@ -88,17 +81,12 @@ try: return repo.scm_instance.get_changeset(rev).raw_id except EmptyRepositoryError, e: - if not redirect_after: - return None h.flash(h.literal(_('There are no changesets yet')), - category='warning') - redirect(url('summary_home', repo_name=repo.repo_name)) - + category='error') + raise HTTPNotFound() except RepositoryError, e: log.error(traceback.format_exc()) - h.flash(safe_str(e), category='warning') - if not partial: - redirect(h.url('summary_home', repo_name=repo.repo_name)) + h.flash(safe_str(e), category='error') raise HTTPBadRequest() @staticmethod @@ -256,8 +244,8 @@ h.flash(msg, category='error') return redirect(url('compare_home', repo_name=c.repo_name)) - c.org_rev = self.__get_rev_or_redirect(ref=org_ref, repo=org_repo, partial=partial) - c.other_rev = self.__get_rev_or_redirect(ref=other_ref, repo=other_repo, partial=partial) + c.org_rev = self.__get_rev(ref=org_ref, repo=org_repo) + c.other_rev = self.__get_rev(ref=other_ref, repo=other_repo) c.compare_home = False c.org_repo = org_repo
--- a/kallithea/controllers/files.py Fri Jul 18 19:21:56 2014 +0200 +++ b/kallithea/controllers/files.py Fri Jul 18 19:22:01 2014 +0200 @@ -72,19 +72,19 @@ super(FilesController, self).__before__() c.cut_off_limit = self.cut_off_limit - def __get_cs_or_redirect(self, rev, repo_name, redirect_after=True): + def __get_cs(self, rev, silent_empty=False): """ Safe way to get changeset if error occur it redirects to tip with proper message :param rev: revision to fetch - :param repo_name: repo name to redirect after + :silent_empty: return None if repository is empty """ try: return c.db_repo_scm_instance.get_changeset(rev) except EmptyRepositoryError, e: - if not redirect_after: + if silent_empty: return None url_ = url('files_add_home', repo_name=c.repo_name, @@ -92,7 +92,7 @@ add_new = h.link_to(_('Click here to add new file'), url_, class_="alert-link") h.flash(h.literal(_('There are no files yet. %s') % add_new), category='warning') - redirect(h.url('summary_home', repo_name=repo_name)) + raise HTTPNotFound() except(ChangesetDoesNotExistError, LookupError), e: log.error(traceback.format_exc()) msg = _('Such revision does not exist for this repository') @@ -102,12 +102,10 @@ h.flash(safe_str(e), category='error') raise HTTPNotFound() - def __get_filenode_or_redirect(self, repo_name, cs, path): + def __get_filenode(self, cs, path): """ - Returns file_node, if error occurs or given path is directory, - it'll redirect to top level path + Returns file_node or raise HTTP error. - :param repo_name: repo_name :param cs: given changeset :param path: path to lookup """ @@ -134,10 +132,10 @@ # redirect to given revision from form if given post_revision = request.POST.get('at_rev', None) if post_revision: - cs = self.__get_cs_or_redirect(post_revision, repo_name) + cs = self.__get_cs(post_revision) c.revision = revision - c.changeset = self.__get_cs_or_redirect(revision, repo_name) + c.changeset = self.__get_cs(revision) c.branch = request.GET.get('branch', None) c.f_path = f_path c.annotate = annotate @@ -200,7 +198,7 @@ 'repository.admin') @jsonify def history(self, repo_name, revision, f_path): - changeset = self.__get_cs_or_redirect(revision, repo_name) + changeset = self.__get_cs(revision) f_path = f_path _file = changeset.get_node(f_path) if _file.is_file(): @@ -223,7 +221,7 @@ @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', 'repository.admin') def authors(self, repo_name, revision, f_path): - changeset = self.__get_cs_or_redirect(revision, repo_name) + changeset = self.__get_cs(revision) f_path = f_path _file = changeset.get_node(f_path) if _file.is_file(): @@ -237,8 +235,8 @@ @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', 'repository.admin') def rawfile(self, repo_name, revision, f_path): - cs = self.__get_cs_or_redirect(revision, repo_name) - file_node = self.__get_filenode_or_redirect(repo_name, cs, f_path) + cs = self.__get_cs(revision) + file_node = self.__get_filenode(cs, f_path) response.content_disposition = 'attachment; filename=%s' % \ safe_str(f_path.split(Repository.url_sep())[-1]) @@ -250,8 +248,8 @@ @HasRepoPermissionAnyDecorator('repository.read', 'repository.write', 'repository.admin') def raw(self, repo_name, revision, f_path): - cs = self.__get_cs_or_redirect(revision, repo_name) - file_node = self.__get_filenode_or_redirect(repo_name, cs, f_path) + cs = self.__get_cs(revision) + file_node = self.__get_filenode(cs, f_path) raw_mimetype_mapping = { # map original mimetype to a mimetype used for "show as raw" @@ -318,8 +316,8 @@ r_post = request.POST - c.cs = self.__get_cs_or_redirect(revision, repo_name) - c.file = self.__get_filenode_or_redirect(repo_name, c.cs, f_path) + c.cs = self.__get_cs(revision) + c.file = self.__get_filenode(c.cs, f_path) c.default_message = _('Deleted file %s via Kallithea') % (f_path) c.f_path = f_path @@ -378,8 +376,8 @@ r_post = request.POST - c.cs = self.__get_cs_or_redirect(revision, repo_name) - c.file = self.__get_filenode_or_redirect(repo_name, c.cs, f_path) + c.cs = self.__get_cs(revision) + c.file = self.__get_filenode(c.cs, f_path) if c.file.is_binary: return redirect(url('files_home', repo_name=c.repo_name, @@ -433,8 +431,7 @@ repo_name=repo_name, revision='tip')) r_post = request.POST - c.cs = self.__get_cs_or_redirect(revision, repo_name, - redirect_after=False) + c.cs = self.__get_cs(revision, silent_empty=True) if c.cs is None: c.cs = EmptyChangeset(alias=c.db_repo_scm_instance.alias) c.default_message = (_('Added file via Kallithea')) @@ -799,7 +796,7 @@ @jsonify def nodelist(self, repo_name, revision, f_path): if request.environ.get('HTTP_X_PARTIAL_XHR'): - cs = self.__get_cs_or_redirect(revision, repo_name) + cs = self.__get_cs(revision) _d, _f = ScmModel().get_nodes(repo_name, cs.raw_id, f_path, flat=False) return {'nodes': _d + _f}
--- a/kallithea/lib/base.py Fri Jul 18 19:21:56 2014 +0200 +++ b/kallithea/lib/base.py Fri Jul 18 19:22:01 2014 +0200 @@ -33,13 +33,14 @@ import traceback from paste.auth.basic import AuthBasicAuthenticator -from paste.httpexceptions import HTTPUnauthorized, HTTPForbidden +from paste.httpexceptions import HTTPUnauthorized, HTTPForbidden, HTTPNotFound from paste.httpheaders import WWW_AUTHENTICATE, AUTHORIZATION from pylons import config, tmpl_context as c, request, session, url from pylons.controllers import WSGIController from pylons.controllers.util import redirect from pylons.templating import render_mako as render # don't remove this import +from pylons.i18n.translation import _ from kallithea import __version__, BACKENDS @@ -408,8 +409,10 @@ if c.db_repo_scm_instance is None: log.error('%s this repository is present in database but it ' 'cannot be created as an scm instance', c.repo_name) - - redirect(url('home')) + from kallithea.lib import helpers as h + h.flash(h.literal(_('Repository not found in the filesystem')), + category='error') + raise HTTPNotFound() # some globals counter for menu c.repository_followers = self.scm_model.get_followers(dbr)