Merge ~andersson123/autopkgtest-cloud:user-specific-page into autopkgtest-cloud:master

[Tim Andersson]

[Tim Andersson (andersson123)]

Needs:
- browse-user.html
- refactoring of the code used - it's somewhat inefficient
- testing

[Tim Andersson (andersson123)]

not sure the refactoring will be viable now, since the semantics of this page are different to other results pages.

This is also now tested in staging.

[Tim Andersson (andersson123)]

This is all fully amended now and ready for re-review

[Tim Andersson (andersson123)]

all tested, see:
https://autopkgtest.staging.ubuntu.com/user/andersson123

Just need to maybe add a section that redirects to this on the navbar, not sure about that yet.

[Tim Andersson (andersson123)]

also need to add a button to go back to the top as well

[Tim Andersson (andersson123)]

Need to also look into having the requester as an index

[Tim Andersson (andersson123)]

session from request.cgi isn't shared with browse.cgi - so adding the button to the navbar isn't easy right now.

We need to add the secret key setup (and session from flask) to browse.cgi - then users logged in via request.cgi will have their session saved in browse.cgi, and the navbar section will be easier to implement.

[Tim Andersson (andersson123)]

I added an index for the requester but it didn't improve load speed :/

 CREATE INDEX result_requester_idx ON result(requester);

altho actually I didn't do it in the public db lol so I should do that too and test again

[Tim Andersson (andersson123)]

yeah, adding the index on the public db helped, I'll amend the mp to include this in init_db

[Tim Andersson (andersson123)]

Sharing the flask session worked immediately ... amazing

[Tim Andersson (andersson123)]

Commit message needs amending but this is ready for review.

[Skia (hyask)]

First round of comments

review: Needs Fixing

[Tim Andersson (andersson123)]

I've addressed all comments apart from the refactoring of browse-user.html and explained my reasoning in an inline comment.

[Tim Andersson (andersson123)]

Okay, I think I've actually finalised this now. It's ready for review.

[Tim Andersson (andersson123)]

Tim to split this into two commits.

ff445d3... by Tim Andersson on 2024-03-27

feat: web: add a user specific history page

Fixes bug LP: #2057947

This commit adds a new endpoint to browse.cgi: /user/<username>

This endpoint shows all the running, queued and historical tests for a
specific user.

When the user is logged in, the login button will display "User".
This button will redirect to the /user/<username> page for the currently
logged in user.

This commit also adds a new line into helpers/utils.py - init_db.
The new line simply creates an index based on the requester column of
the db, in order to speed up the loading of the /user/<username>
endpoint.

This commit also creates a jinja macro called set_up_results_table
which sets up the columns for viewing test results.

It also includes a second macro called results_table_core, which
avoids duplicate html between browse-user and browse-results.

[Tim Andersson (andersson123)]

This has now been split into two commits :)

[Tim Andersson (andersson123)]

and now this mp depends on the following MP:
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/463211

[Tim Andersson (andersson123)]

I need to further the jinja macro-isation and split the user page function into multiple functions to reduce the cyclomatic complexity.

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

make sure session doesn't interact with the user param specified in the request

[Tim Andersson (andersson123)]

User landing page link to be removed

[Tim Andersson (andersson123)]

if I login as hyask and go to https://autopkgtest.staging.ubuntu.com/user/andersson123, the "Next page of Results" link is wrong

[Tim Andersson (andersson123)]

amended based on all comments, please re-review

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

Amended and ready for re-review

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

amended based on comments, I'll test this and request another review.

[Tim Andersson (andersson123)]

TIM: FIX ALL THE VARIABLE SPACES IN TEMPLATES!

[Tim Andersson (andersson123)]

also add buttons for running only, queued only, historical only

[Tim Andersson (andersson123)]

all of this is done and it's ready for re-review.

[Skia (hyask)]

A few inline comments, we're almost there. :-)

review: Needs Fixing

7ebbaac... by Tim Andersson on 2024-04-17

refactor: web: make browse-results.html use the results_table_core macro

[Tim Andersson (andersson123)]

amended and testing now

[Tim Andersson (andersson123)]

tested, fully functional, found a little bug too which I fixed, ready for re-review

[Skia (hyask)]

review: Needs Fixing

[Tim Andersson (andersson123)]

spent some time breaking the page, I'm about to push a couple of fixes and then this is ready for review again

Unmerged commits

7ebbaac... by Tim Andersson on 2024-04-17

refactor: web: make browse-results.html use the results_table_core macro

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

1 → 3 of 3 results

First • Previous • Next • Last

ff445d3... by Tim Andersson on 2024-03-27

feat: web: add a user specific history page

Fixes bug LP: #2057947

This commit adds a new endpoint to browse.cgi: /user/<username>

This endpoint shows all the running, queued and historical tests for a
specific user.

When the user is logged in, the login button will display "User".
This button will redirect to the /user/<username> page for the currently
logged in user.

This commit also adds a new line into helpers/utils.py - init_db.
The new line simply creates an index based on the requester column of
the db, in order to speed up the loading of the /user/<username>
endpoint.

This commit also creates a jinja macro called set_up_results_table
which sets up the columns for viewing test results.

It also includes a second macro called results_table_core, which
avoids duplicate html between browse-user and browse-results.

diff --git a/charms/focal/autopkgtest-web/webcontrol/browse.cgi b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
index 309fb82..16cafea 100755
--- a/charms/focal/autopkgtest-web/webcontrol/browse.cgi
+++ b/charms/focal/autopkgtest-web/webcontrol/browse.cgi
@@ -54,6 +54,7 @@ def init_config():
         uri=True,
         check_same_thread=False,
     )
+    db_con.row_factory = sqlite3.Row
     try:
         url = cp["web"]["ExternalURL"]
     except KeyError:
@@ -62,6 +63,22 @@ def init_config():
     swift_container_url = os.path.join(url, "autopkgtest-%s")
 
 
+def get_package_release_arch(test_id):
+    """
+    The opposite of get_test_id - gets the package/release/arch
+    combo for a given test_id.
+    """
+    c = db_con.cursor()
+    c.execute(
+        "SELECT package, release, arch FROM test WHERE id=?",
+        (test_id,),
+    )
+    try:
+        return c.fetchone()
+    except TypeError:
+        return None
+
+
 def get_test_id(release, arch, src):
     c = db_con.cursor()
     c.execute(
@@ -256,6 +273,128 @@ def success_count_for_release_and_arch(db, release, arch, src_versions):
     return count
 
 
+def get_historical_results_for_user(
+    user: str, limit: int, offset: int
+) -> list:
+    historical_results = []
+    for row in db_con.execute(
+        "SELECT test_id, run_id, version, triggers, "
+        "duration, exitcode, requester, env, uuid FROM result "
+        "WHERE requester=? "
+        "ORDER BY run_id DESC "
+        "LIMIT ? OFFSET ? ",
+        (user, limit, offset),
+    ):
+        test_id = row["test_id"]
+        version = row["version"]
+        triggers = row["triggers"]
+        additional_params = row[
+            "env"
+        ]  # string of comma separated env variables e.g. all-proposed=1,test-name=mytest
+        code = human_exitcode(row["exitcode"])
+        package, release, arch = get_package_release_arch(test_id)
+        url = os.path.join(
+            swift_container_url % release,
+            release,
+            arch,
+            srchash(package),
+            package,
+            row["run_id"],
+        )
+        show_retry = code != "pass"
+        all_proposed = (
+            additional_params is not None
+            and "all-proposed=1" in additional_params
+        )
+        historical_results.append(
+            (
+                version,
+                triggers,
+                additional_params,
+                human_date(row["run_id"]),
+                human_sec(int(row["duration"])),
+                user,
+                code,
+                url,
+                show_retry,
+                all_proposed,
+                row["uuid"],
+                package,
+                release,
+                arch,
+            )
+        )
+    return historical_results
+
+
+def get_queued_results_for_user(user: str):
+    queued_results = []
+    (_, _, queues_info) = get_queues_info()
+    for _, queue in queues_info.items():
+        for release, queue_by_arch in queue.items():
+            for arch, queue_items in queue_by_arch.items():
+                if queue_items[0] == 0:
+                    continue
+                requests = queue_items[1]
+                for req in requests:
+                    try:
+                        req_info = json.loads(req.split("\n")[1])
+                    except json.JSONDecodeError as _:
+                        continue
+                    package = req.split("\n")[0]
+                    if req_info.get("requester", "") == user:
+                        queued_results.append(
+                            (
+                                "N/A",
+                                req_info.get("triggers"),
+                                "N/A",
+                                human_date(req_info.get("submit-time")),
+                                "N/A",
+                                user,
+                                "queued",
+                                "",
+                                False,
+                                "",
+                                req_info.get("uuid", ""),
+                                package,
+                                release,
+                                arch,
+                            ),
+                        )
+    return queued_results
+
+
+def get_running_results_for_user(user: str):
+    running_results = []
+    for package, running_hash in get_running_jobs().items():
+        for _, running in running_hash.items():
+            for release, vals in running.items():
+                for arch, list_of_running_items in vals.items():
+                    if len(list_of_running_items) < 1:
+                        continue
+                    info_dict = list_of_running_items[0]
+                    if info_dict.get("requester", "") == user:
+                        running_results.append(
+                            (
+                                "N/A",
+                                info_dict.get("triggers"),
+                                "N/A",
+                                human_date(info_dict.get("submit-time")),
+                                human_sec(int(list_of_running_items[1])),
+                                user,
+                                "running",
+                                "",
+                                False,
+                                "",
+                                info_dict.get("uuid", "-"),
+                                package,
+                                release,
+                                arch,
+                            ),
+                        )
+    return running_results
+
+
 @app.route("/")
 def index_root():
     flask.session.permanent = True
@@ -344,6 +483,86 @@ def package_overview(package, _=None):
     )
 
 
[e-mail]("/user/<user>")
+def user_overview(user):
+    """
+    This endpoint provides a "per-user" view for autopkgtest-cloud.
+    It shows a page, much like the package/release/arch pages,
+    except all of the queued, running and historical results are
+    only shown if they were requested by the user provided.
+    """
+    user = user.split("&")[0] if "&" in user else user
+
+    def is_it_true(test_string):
+        mapping_dict = {
+            "true": True,
+            "false": False,
+        }
+        return mapping_dict.get(test_string.lower(), True)
+
+    args = flask.request.args
+    # make sure this works with showing all historical results
+    try:
+        limit = int(args.get("limit", 100))
+        assert isinstance(limit, int)
+        assert limit > 0
+        assert limit <= 100
+    except AssertionError:
+        limit = 100
+    try:
+        offset = int(args.get("offset", 0))
+        assert offset > 0
+        assert isinstance(offset, int)
+    except AssertionError:
+        offset = 0
+
+    show_running = args.get("show-running", True, type=is_it_true)
+    show_queued = args.get("show-queued", True, type=is_it_true)
+    show_historical = args.get("show-historical", True, type=is_it_true)
+    only_historical = args.get("only-historical", False, type=is_it_true)
+    if only_historical:
+        limit = 10000
+        offset = 0
+        show_running = False
+        show_queued = False
+        show_historical = (
+            True  # just incase anyone tries fumbling with the args!
+        )
+
+    # Get historical results for this user
+    if show_historical:
+        historical_results = get_historical_results_for_user(
+            user, limit, offset
+        )
+    else:
+        historical_results = []
+
+    # add queued tests for this user
+    if show_queued:
+        queued_results = get_queued_results_for_user(user)
+    else:
+        queued_results = []
+
+    # add running tests from this user
+    if show_running:
+        running_results = get_running_results_for_user(user)
+    else:
+        running_results = []
+
+    return render(
+        "browse-user.html",
+        running_results=running_results,
+        queued_results=queued_results,
+        historical_results=historical_results,
+        limit=limit,
+        offset=offset,
+        show_running=show_running,
+        show_queued=show_queued,
+        show_historical=show_historical,
+        user=user,
+    )
+
+
 # backwards-compatible path with debci that specifies the source hash
 @app.route("/packages/<_>/<package>/<release>/<arch>")
 @app.route("/packages/<package>/<release>/<arch>")
diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
index 4e26eb8..1b2a298 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
@@ -137,6 +137,11 @@ def init_db(path, **kwargs):
             "CREATE INDEX IF NOT EXISTS result_run_ix ON result("
             "  run_id desc)"
         )
+        # /user/<username> page benefits greatly from this idx
+        # Prior to idx, this page would take ~90s to load, down to ~5s.
+        c.execute(
+            "CREATE INDEX IF NOT EXISTS result_requester_idx ON result(requester) "
+        )
         # /admin mostly benefits from the index on test_id (~80s -> 50ms)
         # /packages/<name> also sees some improvements (~14s -> 30ms)
         c.execute(
diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-layout.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-layout.html
index e4cfce6..f3ffd2f 100644
--- a/charms/focal/autopkgtest-web/webcontrol/templates/browse-layout.html
+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-layout.html
@@ -36,6 +36,8 @@
               <li><a href="{{base_url}}admin">Admin</a></li>
               {% if not session.get("nickname") %}
                 <li><a href="{{base_url}}request.cgi?/login">Login</a></li>
+              {% else %}
+                <li><a href="{{ base_url }}user/{{ session.get("nickname") }}">User</a></li>
               {% endif %}
             </ul>
           </div><!--/.nav-collapse -->
diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
index fadff6d..4b72ab2 100644
--- a/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-results.html
@@ -14,50 +14,12 @@
 
   <table class="table">
     <tr>
-      <td><b>Version</b></td>
-      <td><b>Triggers</b></td>
-      <td><b>Env</b></td>
-      <td><b>Date</b></td>
-      <td><b>Duration</b></td>
-      <td><b>Requester</b></td>
-      <td><b>Result</b></td>
-      <td><b>UUID</b></td>
-      <td></td>
+      {{ macros.set_up_results_table() }}
     </tr>
 
   {% for row in package_results %}
     <tr {% if row[6] in ["running", "queued"] %}class="unfinished"{% endif %}>
-      <td>{{row[0]}}</td>
-      <td>{{row[1]}}</td>
-      <td>{{row[2]}}</td>
-      <td>{{row[3]}}</td>
-      <td>{{row[4]}}</td>
-      <td>
-        {% if row[5] != "-" %}
-          <a href="https://launchpad.net/~{{row[5]}}">{{row[5]}}</a>
-        {% else %}
-          {{row[5]}}
-        {% endif %}
-      </td>
-      <td class="nowrap {{row[6]}}" title={{ row[6] }}>{{row[6]}}</td>
-      <td>{{row[10]}}</td>
-      <td class="nowrap">
-        {% if row[6] not in ["running", "queued"] %}
-        <a href="{{row[7]}}/log.gz">log</a> &emsp;
-        <a href="{{row[7]}}/artifacts.tar.gz">artifacts</a> &emsp;
-        {% endif %}
-      </td>
-      <td class="nowrap">
-        {% if row[8] %}
-          {% set ts = row[1].split()|map('urlencode')|join("&trigger=")|safe %}
-          {% set all_proposed = row[9] %}
-          {% if all_proposed %}
-            <a href="{{base_url}}request.cgi?release={{release}}&arch={{arch}}&package={{package|urlencode}}&trigger={{ts}}&all-proposed=1">&#9851;</a>
-          {% else %}
-            <a href="{{base_url}}request.cgi?release={{release}}&arch={{arch}}&package={{package|urlencode}}&trigger={{ts}}">&#9851;</a>
-          {% endif %}
-        {% endif %}
-      </td>
+      {{ macros.results_table_core(row[0], row[1], row[2], row[3], row[4], row[5], row[6], row[7], row[8], row[9], row[10]) }}
     </tr>
   {% endfor %}
   </table>
diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/browse-user.html b/charms/focal/autopkgtest-web/webcontrol/templates/browse-user.html
new file mode 100644
index 0000000..2c11010
--- /dev/null
+++ b/charms/focal/autopkgtest-web/webcontrol/templates/browse-user.html
@@ -0,0 +1,73 @@
+
+{% extends "browse-layout.html" %}
+{% import "macros.html" as macros %}
+
+{% macro display_user_test_info(user_results) -%}
+  <table class="table">
+    <tr>
+      <td><b>Package</b></td>
+      <td><b>Release</b></td>
+      <td><b>Arch</b></td>
+      {{ macros.set_up_results_table() }}
+    </tr>
+  {% for row in user_results %}
+    <tr {% if row[6] in ["running", "queued"] %}class="unfinished"{% endif %}>
+      {% set package = row[11] %}
+      {% set release = row[12] %}
+      {% set arch = row[13] %}
+      <td>{{ package }}</td>
+      <td>{{ release }}</td>
+      <td>{{ arch }}</td>
+      {{ macros.results_table_core(row[0], row[1], row[2], row[3], row[4], row[5], row[6], row[7], row[8], row[9], row[10]) }}
+    </tr>
+  {% endfor %}
+  </table>
+{%- endmacro %}
+
+{% block content %}
+  <h1 class="page-header" id="user-home">Running, queued and complete tests by user</h1>
+  <ul class="external-links">
+    <a href="https://launchpad.net/~{{ user }}"><img src="{{ url_for('static', filename='launchpad.ico') }}" class="icon">Launchpad</a>
+  </ul>
+  <p>Click the link to jump to those specific tests</p>
+
+  {% if show_running %}
+    <h3><a href="#results-running">Running</a></h3>
+    <h4><a href="{{ base_url }}user/{{ user }}?show-running=true&show-queued=false&show-historical=false">Show Running Only</a></h4>
+  {% endif %}
+  {% if show_queued %}
+    <h3><a href="#results-queued">Queued</a></h3>
+    <h4><a href="{{ base_url }}user/{{ user }}?show-running=false&show-queued=true&show-historical=false">Show Queued Only</a></h4>
+  {% endif %}
+  {% if show_historical %}
+    <h3><a href="#results-complete">Complete</a></h3>
+    <h4><a href="{{ base_url }}user/{{ user }}?only-historical=true">Show Historical Only</a></h4>
+  {% endif %}
+
+  {% if show_running %}
+    <h2 id="results-running">Running</h2>
+    <h4><a href="#user-home">Back to top</a></h4>
+    {{ display_user_test_info(running_results)}}
+  {% endif %}
+
+  {% if show_queued %}
+    <h2 id="results-queued">Queued</h2>
+    <h4><a href="#user-home">Back to top</a></h4>
+    {{ display_user_test_info(queued_results)}}
+  {% endif %}
+
+  {% if show_historical %}
+    <h2 id="results-complete">Complete</h2>
+    <h4><a href="#user-home">Back to top</a></h4>
+    {{ display_user_test_info(historical_results)}}
+
+    {% if limit is not none %}
+      <h4><a href="{{ base_url }}user/{{ user }}?offset={{ offset + limit }}&limit={{ limit }}">Next Page of Results</a></h4>
+      {% if offset != 0 %}
+        <h4><a href="{{ base_url }}user/{{ user }}?offset={{ offset - limit }}&limit={{ limit }}">Previous Page of Results</a></h4>
+      {% endif %}
+      <h4><a href="{{ base_url }}user/{{ user }}?only-historical=true">All Results</a></h4>
+    {% endif %}
+  {% endif %}
+
+{% endblock %}
diff --git a/charms/focal/autopkgtest-web/webcontrol/templates/macros.html b/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
index 941dc77..36809a4 100644
--- a/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
+++ b/charms/focal/autopkgtest-web/webcontrol/templates/macros.html
@@ -49,6 +49,51 @@
   {%- endfor %}
 {%- endmacro %}
 
+{% macro set_up_results_table() -%}
+  <td><b>Version</b></td>
+  <td><b>Triggers</b></td>
+  <td><b>Env</b></td>
+  <td><b>Date</b></td>
+  <td><b>Duration</b></td>
+  <td><b>Requester</b></td>
+  <td><b>Result</b></td>
+  <td><b>UUID</b></td>
+  <td></td>
+{%- endmacro %}
+
+{% macro results_table_core(version, triggers, env, date, duration, requester, code, url, show_retry, all_proposed, uuid) -%}
+  <td>{{ version }}</td>
+  <td>{{ triggers }}</td>
+  <td>{{ env }}</td>
+  <td>{{ date }}</td>
+  <td>{{ duration }}</td>
+  <td>
+    {% if requester != "-" %}
+      <a href="https://launchpad.net/~{{ requester }}">{{ requester }}</a>
+    {% else %}
+      {{ requester }}
+    {% endif %}
+  </td>
+  <td class="nowrap {{ code }}" title={{ code }}>{{ code }}</td>
+  <td>{{uuid}}</td>
+  <td class="nowrap">
+    {% if code not in ["running", "queued"] %}
+    <a href="{{ url }}/log.gz">log</a> &emsp;
+    <a href="{{ url }}/artifacts.tar.gz">artifacts</a> &emsp;
+    {% endif %}
+  </td>
+  <td class="nowrap">
+    {% if show_retry %}
+      {% set ts = triggers.split()|map('urlencode')|join("&trigger=")|safe %}
+      {% if all_proposed %}
+        <a href="{{ base_url }}request.cgi?release={{ release }}&arch={{ arch }}&package={{ package|urlencode }}&trigger={{ ts }}&all-proposed=1">&#9851;</a>
+      {% else %}
+        <a href="{{ base_url }}request.cgi?release={{ release }}&arch={{ arch }}&package={{ package|urlencode }}&trigger={{ ts }}">&#9851;</a>
+      {% endif %}
+    {% endif %}
+  </td>
+{%- endmacro %}
+
 {% macro excuses_link(package_name, release="") -%}
 {% if release != "" %}
 {% set release = release + "/" %}