Merge pull request #1470 from ScilifelabDataCentre/dev

New release
ScilifelabDataCentre · Sep 25, 2023 · 7f3e488 · 7f3e488
2 parents d686af2 + 5a8f956
commit 7f3e488
Show file tree

Hide file tree

Showing 25 changed files with 1,495 additions and 582 deletions.
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
@@ -1,76 +1,84 @@
-## Before submitting this PR
+## Read this before submitting the PR
 
-1. **Description:** _Add a summary of the changes in this PR and the related issue._
-2. **Jira task / GitHub issue:** _Link to the github issue or add the Jira task ID here._
-3. **How to test:** _Add information on how someone could manually test this functionality. As detailed as possible._
-4. **Type of change:** [_Check the relevant boxes in the section below_](#what-type-of-changes-does-the-pr-contain)
-5. **Add docstrings and comments to code**, _even if_ you personally think it's obvious.
+1. Always create a Draft PR first
+2. Go through sections 1-5 below, fill them in and check all the boxes
+3. Make sure that the branch is updated; if there's an "Update branch" button at the bottom of the PR, rebase or update branch.
+4. When all boxes are checked, information is filled in, and the branch is updated: mark as Ready For Review and tag reviewers (top right)
+5. Once there is a submitted review, implement the suggestions (if reasonable, otherwise discuss) and request an new review.
 
-## What _type of change(s)_ does the PR contain?
+If there is a field which you are unsure about, enter the edit mode of this description or go to the [PR template](../.github/pull_request_template.md); There are invisible comments providing descriptions which may be of help.
 
-<!--
-- "Breaking": The change will cause existing functionality to not work as expected.
-- Workflow: E.g. a new github action or changes to this PR template. Anything that alters our or the codes workflow.
--->
+## 1. Description / Summary
+
+_Add a summary of the changes in this PR and the related issue._
+
+## 2. Jira task / GitHub issue
+
+_Link to the github issue or add the Jira task ID here._
+
+## 3. Type of change
+
+What _type of change(s)_ does the PR contain?
+
+**Check the relevant boxes below. For an explanation of the different sections, enter edit mode of this PR description template.**
 
 - [ ] New feature
-  - [ ] Breaking: _Please describe the reason for the break and how we can fix it._
-  - [ ] Non-breaking
-- [ ] Database change
-  - [ ] Migration _included in PR_
-  - [ ] Migration _not needed_
-- [ ] Bug fix
-  - [ ] Breaking: _Please describe the reason for the break and how we can fix it._
-  - [ ] Non-breaking
-- [ ] Security Alert fix
-- [ ] Documentation
-- [ ] Tests **(only)**
-- [ ] Workflow
-
-## Checklist
-
-- [Sprintlog](../SPRINTLOG.md)
-  - [ ] Added
-  - [ ] Not needed (E.g. PR contains _only_ tests)
-- Rebase / Update / Merge _from_ base branch (the branch from which the current is forked)
-  - [ ] Done
-  - [ ] Not needed
-- Blocking PRs
-  - [ ] Merged
-  - [ ] No blocking PRs
-- PR to `master` branch
-  - [ ] Yes: Read [the release instructions](https://github.com/ScilifelabDataCentre/dds_web/blob/master/doc/procedures/new_release.md)
-    - [ ] I have followed steps 1-8.
-  - [ ] No
-
-## Actions / Scans
-
-<!-- Go through all checkboxes. All actions must pass before merging is allowed.-->
-
-- **Black**: Python code formatter. Does not execute. Only tests.
-  Run `black .` locally to execute formatting.
-  - [ ] Passed
-- **Prettier**: General code formatter. Our use case: MD and yaml mainly.
-  Run `npx prettier --write .` locally to execute formatting.
-  - [ ] Passed
-- **Yamllint**: Linting of yaml files.
-  - [ ] Passed
-- **Tests**: Pytest to verify that functionality works as expected.
-  - [ ] New tests added
-  - [ ] No new tests
-  - [ ] Passed
-- **CodeQL**: Scan for security vulnerabilities, bugs, errors
-  - [ ] New alerts: _Go through them and either fix, dismiss och ignore. Add reasoning in items below._
-  - [ ] Alerts fixed: _What?_
-  - [ ] Alerts ignored / dismissed: _Why?_
-  - [ ] Passed
-- **Trivy**: Security scanner
-  - [ ] New alerts: _Go through them and either fix, dismiss och ignore. Add reasoning in items below._
-  - [ ] Alerts fixed: _What?_
-  - [ ] Alerts ignored / dismissed: _Why?_
-  - [ ] Passed
-- **Snyk**: Security scanner
-  - [ ] New alerts: _Go through them and either fix, dismiss och ignore. Add reasoning in items below._
-  - [ ] Alerts fixed: _What?_
-  - [ ] Alerts ignored / dismissed: _Why?_
-  - [ ] Passed
+  - [ ] Breaking: _Why / How? Add info here._ <!-- Should be checked if the changes in this PR will cause existing functionality to not work as expected. E.g. with the master branch of the `dds_cli` -->
+  - [ ] Non-breaking <!-- Should be checked if the changes will not cause existing functionality to fail. "Non-breaking" is just an addition of a new feature. -->
+- [ ] Database change: _Remember the to include a new migration version, **or** explain here why it's not needed._ <!-- Should be checked when you've changed something in `models.py`. For a guide on how to add the a new migration version, look at the "Database changes" section in the README.md. -->
+- [ ] Bug fix <!-- Should be checked when a bug is fixed in existing functionality. If the bug fix also is a breaking change (see above), add info about that beside this check box. -->
+- [ ] Security Alert fix <!-- Should be checked if the PR attempts to solve a security vulnerability, e.g. reported by the "Security" tab in the repo. -->
+- [ ] Documentation <!-- Should be checked if the PR adds or updates documentation such as e.g. Technical Overview or a architecture decision (dds_web/doc/architecture/decisions.) -->
+- [ ] Workflow <!-- Should be checked if the PR includes a change in e.g. the github actions files (dds_web/.github/*) or another type of workflow change. Anything that alters our or the codes workflow. -->
+- [ ] Tests **only** <!-- Should only be checked if the PR only contains tests, none of the other types of changes listed above. -->
+
+## 4. Additional information
+
+- [ ] [Sprintlog](../SPRINTLOG.md) <!-- Add a row at the bottom of the SPRINTLOG.md file (not needed if PR contains only tests). Follow the format of previous rows. If the PR is the first in a new sprint, add a new sprint header row (follow the format of previous sprints). -->
+- [ ] Blocking PRs <!-- Should be checked if there are blocking PRs or other tasks that need to be merged prior to this. Add link to PR or Jira card if this is the case. -->
+  - [ ] Merged <!-- Should be checked if the "Blocking PRs" box was checked AND all blocking PRs have been merged / fixed. -->
+- [ ] PR to `master` branch: \_If checked, read [the release instructions](../doc/procedures/new_release.md) <!-- Check this if the PR is made to the `master` branch. Only the `dev` branch should be doing this. -->
+  - [ ] I have followed steps 1-8. <!-- Should be checked if the "PR to `master` branch" box is checked AND the specified steps in the release instructions have been followed. -->
+
+## 5. Actions / Scans
+
+_Check the boxes when the specified checks have passed._
+
+**For information on what the different checks do and how to fix it if they're failing, enter edit mode of this description or go to the [PR template](../.github/pull_request_template.md).**
+
+- [ ] **Black**
+<!--
+  What: Python code formatter.
+  How to fix: Run `black .` locally to execute formatting.
+-->
+- [ ] **Prettier**
+<!--
+  What: General code formatter. Our use case: MD and yaml mainly.
+  How to fix: Run npx prettier --write . locally to execute formatting.
+-->
+- [ ] **Yamllint**
+<!--
+  What: Linting of yaml files.
+  How to fix: Manually fix any errors locally.
+-->
+- [ ] **Tests**
+<!--
+  What: Pytest to verify that functionality works as expected.
+  How to fix: Manually fix any errors locally. Follow the instructions in the "Run tests" section of the README.md to run the tests locally.
+  Additional info: The PR should ALWAYS include new tests or fixed tests when there are code changes. When pytest action has finished, it will post a codecov report; Look at this report and verify the files you have changed are listed. "90% <100.00%> (+0.8%)" means "Tests cover 90% of the changed file, <100 % of this PR's code changes are tested>, and (the code changes and added tests increased the overall test coverage with 0.8%)
+-->
+- [ ] **CodeQL**
+<!--
+  What: Scan for security vulnerabilities, bugs, errors.
+  How to fix: Go through the alerts and either manually fix, dismiss or ignore. Add info on ignored or dismissed alerts.
+-->
+- [ ] **Trivy**
+<!--
+  What: Security scanner.
+  How to fix: Go through the alerts and either manually fix, dismiss or ignore. Add info on ignored or dismissed alerts.
+-->
+- [ ] **Snyk**
+<!--
+  What: Security scanner.
+  How to fix: Go through the alerts and either manually fix, dismiss or ignore. Add info on ignored or dismissed alerts.
+-->
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -1,9 +1,24 @@
 Changelog
 ==========
 
+.. _2.5.1:
+
+2.5.1 - 2023-09-27
+~~~~~~~~~~~~~~~~~~~
+
+- Super Admins only: 
+    - New endpoint `MaintenanceMode.get`: Super Admins can get info on whether or not the DDS maintenance mode is on or off.
+    - Statistics endpoint returns date of generated statistics, not time.  
+- Bugs fixed:
+    - Errors when attempting to create a project after it has failed due to a database error should now not happen; Database rollback added to project creation endpoint.
+    - Researchers should now always appear in the list of project users after running `dds project access fix --project <proj_id>`; Missing database update added.
+    - Expired invites are deleted automatically when invite is sent to user again; Deleting invite with `dds user delete --is-invite` is no longer necessary prior to a new `dds user add`. 
+- Dependencies:
+    - `MariaDB` from EOL `10.7.8` to LTS `10.11.5`
+
 .. _2.5.0:
 
-version 2.5.0 - 2023-08-30
+2.5.0 - 2023-08-30
 ~~~~~~~~~~~~~~~~~~~~~~~~
 
 - Dependencies: 
@@ -62,4 +77,4 @@ version 2.5.0 - 2023-08-30
 Earlier versions
 ~~~~~~~~~~~~~~~~~
 
-Please see `the release page on GitHub <https://github.com/ScilifelabDataCentre/dds_web/releases>`_ for detailed information about the changes in each release.
+Please see `the release page on GitHub <https://github.com/ScilifelabDataCentre/dds_web/releases>`_ for detailed information about the changes in each release.
diff --git a/SPRINTLOG.md b/SPRINTLOG.md
@@ -290,3 +290,19 @@ _Nothing merged in CLI during this sprint_
 
 - Dependency: Bump `certifi` to 2023.07.22 due to security vulnerability alert(s) ([#1452](https://github.com/ScilifelabDataCentre/dds_web/pull/1452))
 - New version: 2.5.0 ([#1458](https://github.com/ScilifelabDataCentre/dds_web/pull/1458))
+- Added check for Maintenance mode status in MaintenanceMode endpoint ([#1459](https://github.com/ScilifelabDataCentre/dds_web/pull/1459))
+
+# 2023-09-04 - 2023-09-15
+
+- Bug fix: Database rollback added on project creation failure ([#1461](https://github.com/ScilifelabDataCentre/dds_web/pull/1461))
+- Only return date (not time) from `Statistics` endpoint ([#1456](https://github.com/ScilifelabDataCentre/dds_web/pull/1456))
+- Set `sto2*` columns in `Unit` table to nullable ([#1456](https://github.com/ScilifelabDataCentre/dds_web/pull/1462))
+- Dependency: Bump `MariaDB` to LTS version 10.11.5 ([#1465](https://github.com/ScilifelabDataCentre/dds_web/pull/1465))
+- Bug fixed: Row in `ProjectUsers` should also be added if it doesn't exist when giving Researcher access to a specific project ([#1464](https://github.com/ScilifelabDataCentre/dds_web/pull/1464))
+- Workflow: Update PR template and clarify sections ([#1467](https://github.com/ScilifelabDataCentre/dds_web/pull/1467))
+
+# 2023-09-18 - 2023-09-29
+
+- Column `sto4_start_time` is automatically set when the create-unit command is run ([#1668])(https://scilifelab.atlassian.net/jira/software/projects/DDS/boards/13?selectedIssue=DDS-1668)
+- Replace expired invites when there's a new invitation attempt ([#1466](https://github.com/ScilifelabDataCentre/dds_web/pull/1466))
+- New version: 2.5.1 ([#1471](https://github.com/ScilifelabDataCentre/dds_web/pull/1471))
diff --git a/dds_web/api/__init__.py b/dds_web/api/__init__.py
@@ -81,7 +81,7 @@ def output_json(data, code, headers=None):
 
 # Super Admins ###################################################################### Super Admins #
 
-api.add_resource(superadmin_only.SetMaintenance, "/maintenance", endpoint="maintenance")
+api.add_resource(superadmin_only.MaintenanceMode, "/maintenance", endpoint="maintenance")
 api.add_resource(superadmin_only.AllUnits, "/unit/info/all", endpoint="all_units")
 api.add_resource(superadmin_only.MOTD, "/motd", endpoint="motd")
 api.add_resource(superadmin_only.SendMOTD, "/motd/send", endpoint="send_motd")

diff --git a/dds_web/api/project.py b/dds_web/api/project.py
@@ -678,6 +678,8 @@ def post(self):
             new_project = project_schemas.CreateProjectSchema().load(p_info)
             db.session.add(new_project)
         except (sqlalchemy.exc.OperationalError, sqlalchemy.exc.SQLAlchemyError) as err:
+            flask.current_app.logger.info("Doing db rollback.")
+            db.session.rollback()
             raise DatabaseError(message=str(err), alt_message="Unexpected database error.")
 
         if not new_project:
@@ -911,6 +913,20 @@ def give_project_access(project_list, current_user, user):
                         project_id=proj.id, user_id=user.username
                     ).one_or_none()
                     if not project_keys_row:
+                        # Make sure that Researchers are also listed in project users
+                        if (
+                            user.role == "Researcher"
+                            and not models.ProjectUsers.query.filter_by(
+                                project_id=proj.id, user_id=user.username
+                            ).one_or_none()
+                        ):
+                            # New row in association table
+                            new_projectuser_row = models.ProjectUsers(
+                                project_id=proj.id, user_id=user.username
+                            )
+                            # Append association -- only one required, not both ways
+                            proj.researchusers.append(new_projectuser_row)
+
                         share_project_private_key(
                             from_user=current_user,
                             to_another=user,

diff --git a/dds_web/api/schemas/user_schemas.py b/dds_web/api/schemas/user_schemas.py
@@ -4,6 +4,9 @@
 # IMPORTS ################################################################################ IMPORTS #
 ####################################################################################################
 
+# standar library
+import datetime
+
 # Installed
 import flask
 import marshmallow
@@ -72,6 +75,18 @@ def return_invite(self, data, **kwargs):
         # double check if there is no existing user with this email
         userexists = utils.email_in_db(email=data.get("email"))
 
+        # check if the user invite should have expired and be deleted
+        if invite and (
+            (
+                datetime.datetime.utcnow()
+                - datetime.timedelta(hours=flask.current_app.config["INVITATION_EXPIRES_IN_HOURS"])
+            )
+            > invite.created_at
+        ):
+            db.session.delete(invite)
+            db.session.commit()
+            invite = None
+
         if userexists and invite:
             raise ddserr.DatabaseError(message="Email exists for user and invite at the same time")
 

diff --git a/dds_web/api/superadmin_only.py b/dds_web/api/superadmin_only.py
@@ -256,7 +256,7 @@ def put(self):
         }
 
 
-class SetMaintenance(flask_restful.Resource):
+class MaintenanceMode(flask_restful.Resource):
     """Change the maintenance mode of the system."""
 
     @auth.login_required(role=["Super Admin"])
@@ -285,6 +285,15 @@ def put(self):
 
         return {"message": f"Maintenance set to: {setting.upper()}"}
 
+    @auth.login_required(role=["Super Admin"])
+    @logging_bind_request
+    @handle_db_error
+    def get(self):
+        """Return current Maintenance mode."""
+        current_mode = models.Maintenance.query.first()
+
+        return {"message": f"Maintenance mode is set to: {'ON' if current_mode.active else 'OFF'}"}
+
 
 class AnyProjectsBusy(flask_restful.Resource):
     """Check if any projects are busy."""
@@ -325,7 +334,7 @@ def get(self):
         return {
             "stats": [
                 {
-                    "Date": str(row.date),
+                    "Date": str(row.date.date()),
                     "Units": row.unit_count,
                     "Researchers": row.researcher_count,
                     "Project Owners": row.project_owner_unique_count,
@@ -343,7 +352,24 @@ def get(self):
                 }
                 for row in stat_rows
                 if stat_rows
-            ]
+            ],
+            "columns": {
+                "Date": "Date on which the stats were recorded in the database.",
+                "Units": "Number of SciLifeLab units that are using the DDS for data deliveries.",
+                "Researchers": "Number of accounts with the role 'Researcher'.",
+                "Project Owners": "Number of (unique) 'Researcher' accounts with admin permissions in at least one project.",
+                "Unit Personnel": "Number of accounts with the role 'Unit Personnel'.",
+                "Unit Admins": "Number of accounts with the role 'Unit Admin'.",
+                "Super Admins": "Number of employees at the SciLifeLab Data Centre with the DDS account role 'Super Admin'.",
+                "Total Users": "Total number of accounts. Project Owners are a subrole of 'Researchers' and are therefore not included in the summary.",
+                "Total Projects": "Sum of active- and inactive projects.",
+                "Active Projects": "Delivery projects currently used to deliver data. Statuses included are 'In Progress', 'Available' and 'Expired'.",
+                "Inactive Projects": "Delivery projects that have previously been created and/or used for data deliveries. Statuses included are 'Deleted', 'Archived' (incl. aborted).",
+                "Data Now (TB)": "Number of terrabytes of data that are currently being delivered with the DDS.",
+                "Data Uploaded (TB)": "Total number of terrabytes of data that have been uploaded to the DDS temporary storage location since the DDS went into production.",
+                "TBHours Last Month": "Number of terrabyte hours that were recorded in the DDS the previous month. ",
+                "TBHours Total": "Total number of terrabyte hours that have been recorded in the DDS since going into production.",
+            },
         }
 
 

diff --git a/dds_web/api/user.py b/dds_web/api/user.py
@@ -83,8 +83,13 @@ def post(self):
         try:
             existing_user = user_schemas.UserSchema().load({"email": email})
             unanswered_invite = user_schemas.UnansweredInvite().load({"email": email})
-        except sqlalchemy.exc.OperationalError as err:
-            raise ddserr.DatabaseError(message=str(err), alt_message="Unexpected database error.")
+        except (sqlalchemy.exc.SQLAlchemyError, sqlalchemy.exc.OperationalError) as err:
+            db.session.rollback()
+            raise ddserr.DatabaseError(
+                message=str(err),
+                alt_message="Something happened while checking for existig account / active invite.",
+                pass_message=False,
+            )
 
         if existing_user or unanswered_invite:
             if not project:

diff --git a/dds_web/commands.py b/dds_web/commands.py
@@ -106,6 +106,7 @@ def create_new_unit(
     https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
     """
     from dds_web.database import models
+    from dds_web.utils import current_time
 
     error_message = ""
     if len(public_id) > 50:
@@ -131,6 +132,7 @@ def create_new_unit(
         external_display_name=external_display_name,
         contact_email=contact_email,
         internal_ref=internal_ref or public_id,
+        sto4_start_time=current_time(),
         sto4_endpoint=safespring_endpoint,
         sto4_name=safespring_name,
         sto4_access=safespring_access,