Merge ~andersson123/autopkgtest-cloud:fix-package-page-nonexistent-package into autopkgtest-cloud:master

[Tim Andersson]

[Skia (hyask)]

Looking good!

review: Approve

[Brian Murray (brian-murray)]

Thinking about this some more I'm not sure what is the worst end user experience.

A) Mistype a package name and get a web page with no test results.

OR

B) Properly type a package name and get a 404.

I'm leaning towards B being the worst end user experience since at least with A I can click on the LP url and get a nicely formatted 404. Especially, since case A also will not show any releases which is a pretty good indicator you misspelled the package name.

review: Disapprove

[Tim Andersson (andersson123)]

Okay, understood, so, should I close this MP? And the related bug?

[Brian Murray (brian-murray)]

Well ideally the autopkgtest-web workers would know what is and is not a package and then could either return a 404 or a page saying the package has no tests. However, I think that makes the bug a lower priority given the difficulty of it.

[Tim Andersson (andersson123)]

How about 2 outcomes?

outcome 1:
- user types in a package which exists in launchpad but has no previous results - is greeted with a page stating as such
- user types in a package which doesn't exist in launchpad but does have previous results - is greeted with the normal package browsing page
- user types in a package which doesn't exist in launchpad and has no previous results - we then assume the package doesn't exist and we tell the user as such

The above solution is quite easy I believe. Let me know what you think. If you like that solution I can go ahead and implement it.

[Tim Andersson (andersson123)]

waiting on feedback for this MP.

Unmerged commits

ba44a14... by Tim Andersson on 2024-03-19

fix: web: Don't show package pages for packages that don't exist

Fixes bug LP: #2058059

	pre_commit:0 (build)
	unit_tests:0 (build)
	build_charms:0 (build)

1 → 3 of 3 results

First • Previous • Next • Last

diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
index 810163d..4352154 100755
--- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi
+++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
@@ -12,7 +12,7 @@ from wsgiref.handlers import CGIHandler
 
 import flask
 from helpers.admin import select_abnormally_long_jobs
-from helpers.exceptions import RunningJSONNotFound
+from helpers.exceptions import NonExistentPackage, RunningJSONNotFound
 from helpers.utils import get_all_releases, get_supported_releases
 from werkzeug.middleware.proxy_fix import ProxyFix
 
@@ -279,6 +279,8 @@ def package_overview(package, _=None):
     ):
         arches.add(row[3])
         results.setdefault(row[2], {})[row[3]] = human_exitcode(row[1])
+    if not results:
+        raise NonExistentPackage(package)
 
     running_info = dict(
         (k, v) for (k, v) in get_running_jobs().items() if k == package
diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
index e94164e..c3a22d9 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
@@ -15,6 +15,15 @@ class RunningJSONNotFound(FileNotFoundError):
         return 500
 
 
+class NonExistentPackage(Exception):
+    def __init__(self, package):
+        super().__init__("Package %s doesn't exist." % package)
+        self._code = 404
+
+    def exit_code(self):
+        return self._code
+
+
 class WebControlException(Exception):
     def __init__(self, message, exit_code):
         super().__init__(message)