Merge branch 'testing' of github.com:genenetwork/genenetwork2 into testing

12 files changed, 581 insertions, 65 deletions

diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
index 789d6a57..7d7e34a5 100644
--- a/.github/ISSUE_TEMPLATE/bug_report.md
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -26,7 +26,8 @@ If applicable, add screenshots to help explain your problem.
 **Environment setup (please complete the following information):**
 
 - OS: [e.g. Linux]
-- Racket Version [e.g. v7.6]
+- Guix Version (optional)
+- [Anything else you think is relevant]
 
 **Additional context**
 
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
new file mode 100644
index 00000000..4c8db1c4
--- /dev/null
+++ b/.github/workflows/main.yml
@@ -0,0 +1,31 @@
+name: tests
+
+# Run actions when pushing to the testing branch or when you create a
+# PR against it
+on:
+  push:
+    branches: [ testing ]
+  pull_request:
+    branches: [ testing ]
+
+jobs:
+  unittest:
+    runs-on: ubuntu-latest
+    container: bonfacekilz/python2-genenetwork2:latest
+
+    steps:
+    # Use v1 of checkout since v2 fails
+    - name: Checkout Project
+      uses: actions/checkout@v1
+
+    # Redis is required by some of the tests
+    - name: Start Redis
+      run: |
+        redis-server --daemonize yes
+
+    - name: Run the unit tests
+      run: |
+        mkdir -p /genotype_files/genotype/json
+        env GN2_PROFILE=/usr/gn2-profile TMPDIR=/tmp SERVER_PORT=5004 \
+        GENENETWORK_FILES=/genotype_files/ bash bin/genenetwork2 \
+        etc/default_settings.py -c -m unittest discover -v
diff --git a/README.md b/README.md
index 46264252..cd35defb 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,5 @@
 [![DOI](https://zenodo.org/badge/5591/genenetwork/genenetwork2.svg)](https://zenodo.org/badge/latestdoi/5591/genenetwork/genenetwork2) [![JOSS](http://joss.theoj.org/papers/10.21105/joss.00025/status.svg)](http://joss.theoj.org/papers/10.21105/joss.00025)
+[![Actions Status](https://github.com/genenetwork/genenetwork2/workflows/tests/badge.svg)](https://github.com/genenetwork/genenetwork2/actions)
 
 # GeneNetwork
 
@@ -42,9 +43,17 @@ Also mariadb and redis need to be running, see
 ## Testing
 
 To have tests pass, the redis and mariadb instance should be running, because of
-asserts sprinkled in the code base(these will be removed in due time).
+asserts sprinkled in the code base.
+
+Right now, the only tests running in CI are unittests. Please make
+sure the existing unittests are green when submitting a PR.
+
+See
+[./bin/genenetwork2](https://github.com/genenetwork/genenetwork2/blob/testing/doc/docker-container.org)
+for more details.
 
 #### Mechanical Rob
+
 We are building 'Mechanical Rob' automated testing using Python
 [requests](https://github.com/genenetwork/genenetwork2/tree/testing/test/requests)
 which can be run with:
diff --git a/doc/docker-container.org b/doc/docker-container.org
new file mode 100644
index 00000000..3c9864c5
--- /dev/null
+++ b/doc/docker-container.org
@@ -0,0 +1,86 @@
+#+TITLE: Genenetwork2 Dockerized
+
+* Table of Contents                                                     :TOC:
+- [[#introduction][Introduction]]
+- [[#creating-the-docker-images][Creating the Docker Images]]
+- [[#pushing-to-dockerhub][Pushing to DockerHub]]
+
+* Introduction
+
+The CI(Continuous Integration) system for Genenetwork2 uses [[https://github.com/features/actions][Github
+Actions]]. As such, it's important to have a way to run tests using
+facilities provided by GUIX in a reproducible way. This project
+leverages GUIX to generate a docker container from which the unittests
+are ran from.
+
+Find instructions on how to set docker up inside GUIX [[https://github.com/pjotrp/guix-notes/blob/master/CONTAINERS.org#run-docker][here]]. This
+document will not get into that. It's assumed that you have a working
+dockec setup.
+
+The rest of this document outlines how the docker container used in
+the CI builds were created.
+
+* Creating the Docker Images
+
+First create the image by running:
+
+#+begin_src sh
+# For the Python 2 version:
+env GUIX_PACKAGE_PATH="/home/bonface/projects/guix-bioinformatics::/home/bonface/projects/guix-past/modules" \
+    ./pre-inst-env guix pack -f docker --no-grafts \
+    -S /usr/bin=/bin -S /etc/profile=/etc/profile \
+    -S /share/genenetwork2=/share/genenetwork2 \
+    -S /share/javascript=/share/javascript \
+    -S /lib=/lib \
+    -S /usr/gn2-profile=/ \
+    coreutils bash genenetwork2
+
+# For the Python 3 version:
+env GUIX_PACKAGE_PATH="/home/bonface/projects/guix-bioinformatics::/home/bonface/projects/guix-past/modules" \
+    ./pre-inst-env guix pack -f docker --no-grafts \
+    -S /usr/bin=/bin -S /etc/profile=/etc/profile \
+    -S /share/genenetwork2=/share/genenetwork2 \
+    -S /share/javascript=/share/javascript \
+    -S /lib=/lib \
+    -S /usr/gn2-profile=/ \
+    coreutils bash python3-genenetwork2
+  #+end_src
+
+The output will look something similar to:
+
+: /gnu/store/dj1xh19jq1l9vwq24w3nay2954x0wabb-docker-pack.tar.gz
+
+Load the docker image by running:
+
+: docker load --input /gnu/store/dj1xh19jq1l9vwq24w3nay2954x0wabb-docker-pack.tar.gz
+
+Results look something similar to:
+
+#+begin_export ascii
+a93f52b7f565: Loading layer  3.174GB/3.174GB
+Loaded image: coreutils-genenetwork2:latest
+#+end_export
+
+Assuming you have a docker instance running, you could always run
+commands in it e.g:
+
+: docker run "coreutils-genenetwork2:latest" python --version 
+
+
+* Pushing to DockerHub
+
+We use DockerHub to store the docker images from which we use on our
+CI environment using Github Actions.
+
+To push to dockerhub, first get the image name by running =docker
+images=. Push to dockerhub using a command similar to:
+
+: docker push bonfacekilz/python2-genenetwork2:latest
+
+Right now, we have 2 images on DockerHub:
+
+- https://hub.docker.com/repository/docker/bonfacekilz/python2-genenetwork2:
+  Contains the python2 version of gn2. Don't use this. Please use the
+  python3 image!
+- https://hub.docker.com/repository/docker/bonfacekilz/python3-genenetwork2:
+  Contains the python3 version of gn2.
diff --git a/test/requests/main_web_functionality.py b/test/requests/main_web_functionality.py
index d070dab9..d4c3b1ad 100644
--- a/test/requests/main_web_functionality.py
+++ b/test/requests/main_web_functionality.py
@@ -20,7 +20,7 @@ def check_search_page(host):
         , search_terms_or=""
         , search_terms_and="MEAN=(15 16) LRS=(23 46)")
     result = requests.get(host+"/search", params=data)
-    found = result.text.find("records are shown below")
+    found = result.text.find("records were found")
     assert(found >= 0)
     assert(result.status_code == 200)
     print("OK")
diff --git a/wqflask/tests/base/test_data_set.py b/wqflask/tests/base/test_data_set.py
index 94780a5d..dd7f5051 100644
--- a/wqflask/tests/base/test_data_set.py
+++ b/wqflask/tests/base/test_data_set.py
@@ -66,7 +66,7 @@ class TestDataSetTypes(unittest.TestCase):
     @mock.patch('base.data_set.g')
     def test_set_dataset_key_mrna(self, db_mock):
         with app.app_context():
-            db_mock.db.execute.return_value = [1, 2, 3]
+            db_mock.db.execute.return_value.fetchone.return_value = [1, 2, 3]
             redis_mock = mock.Mock()
             redis_mock.get.return_value = self.test_dataset
             data_set = DatasetType(redis_mock)
@@ -84,7 +84,7 @@ class TestDataSetTypes(unittest.TestCase):
     @mock.patch('base.data_set.g')
     def test_set_dataset_key_pheno(self, db_mock):
         with app.app_context():
-            db_mock.db.execute.return_value = [1, 2, 3]
+            db_mock.db.execute.return_value.fetchone.return_value = [1, 2, 3]
             redis_mock = mock.Mock()
             redis_mock.get.return_value = self.test_dataset
             data_set = DatasetType(redis_mock)
@@ -93,7 +93,6 @@ class TestDataSetTypes(unittest.TestCase):
             redis_mock.set.assert_called_once_with(
                 "dataset_structure",
                 '{"Aging-Brain-UCIPublish": "Publish", "AKXDGeno": "Geno", "B139_K_1206_M": "ProbeSet", "AD-cases-controls-MyersGeno": "Geno", "AD-cases-controls-MyersPublish": "Publish", "All Phenotypes": "Publish", "Test": "Publish", "AXBXAPublish": "Publish", "B139_K_1206_R": "ProbeSet", "AXBXAGeno": "Geno"}')
-            expected_db_call = """"""
             db_mock.db.execute.assert_called_with(
                 ("SELECT InfoFiles.GN_AccesionId " +
                  "FROM InfoFiles, PublishFreeze, InbredSet " +
@@ -105,7 +104,7 @@ class TestDataSetTypes(unittest.TestCase):
     @mock.patch('base.data_set.g')
     def test_set_dataset_other_pheno(self, db_mock):
         with app.app_context():
-            db_mock.db.execute.return_value = [1, 2, 3]
+            db_mock.db.execute.return_value.fetchone.return_value = [1, 2, 3]
             redis_mock = mock.Mock()
             redis_mock.get.return_value = self.test_dataset
             data_set = DatasetType(redis_mock)
@@ -114,7 +113,6 @@ class TestDataSetTypes(unittest.TestCase):
             redis_mock.set.assert_called_once_with(
                 "dataset_structure",
                 '{"Aging-Brain-UCIPublish": "Publish", "AKXDGeno": "Geno", "B139_K_1206_M": "ProbeSet", "AD-cases-controls-MyersGeno": "Geno", "AD-cases-controls-MyersPublish": "Publish", "All Phenotypes": "Publish", "Test": "Publish", "AXBXAPublish": "Publish", "B139_K_1206_R": "ProbeSet", "AXBXAGeno": "Geno"}')
-            expected_db_call = """"""
             db_mock.db.execute.assert_called_with(
                 ("SELECT PublishFreeze.Name " +
                  "FROM PublishFreeze, InbredSet " +
@@ -125,7 +123,7 @@ class TestDataSetTypes(unittest.TestCase):
     @mock.patch('base.data_set.g')
     def test_set_dataset_geno(self, db_mock):
         with app.app_context():
-            db_mock.db.execute.return_value = [1, 2, 3]
+            db_mock.db.execute.return_value.fetchone.return_value = [1, 2, 3]
             redis_mock = mock.Mock()
             redis_mock.get.return_value = self.test_dataset
             data_set = DatasetType(redis_mock)
diff --git a/wqflask/tests/base/test_trait.py b/wqflask/tests/base/test_trait.py
new file mode 100644
index 00000000..53b0d440
--- /dev/null
+++ b/wqflask/tests/base/test_trait.py
@@ -0,0 +1,101 @@
+# -*- coding: utf-8 -*-
+"""Tests wqflask/base/trait.py"""
+import unittest
+import mock
+
+from base.trait import GeneralTrait
+from base.trait import retrieve_trait_info
+
+
+class TestResponse:
+    """Mock Test Response after a request"""
+    @property
+    def content(self):
+        """Mock the content from Requests.get(params).content"""
+        return "[1, 2, 3, 4]"
+
+
+class TestNilResponse:
+    """Mock Test Response after a request"""
+    @property
+    def content(self):
+        """Mock the content from Requests.get(params).content"""
+        return "{}"
+
+
+class MockTrait(GeneralTrait):
+    @property
+    def wikidata_alias_fmt(self):
+        return "Mock alias"
+
+
+class TestRetrieveTraitInfo(unittest.TestCase):
+    """Tests for 'retrieve_trait_info'"""
+    def test_retrieve_trait_info_with_empty_dataset(self):
+        """Test that an exception is raised when dataset is empty"""
+        with self.assertRaises(AssertionError):
+            retrieve_trait_info(trait=mock.MagicMock(),
+                                dataset={})
+
+    @mock.patch('base.trait.requests.get')
+    @mock.patch('base.trait.g')
+    def test_retrieve_trait_info_with_empty_trait_info(self,
+                                                       g_mock,
+                                                       requests_mock):
+        """Empty trait info"""
+        requests_mock.return_value = TestNilResponse()
+        with self.assertRaises(KeyError):
+            retrieve_trait_info(trait=mock.MagicMock(),
+                                dataset=mock.MagicMock())
+
+    @mock.patch('base.trait.requests.get')
+    @mock.patch('base.trait.g')
+    def test_retrieve_trait_info_with_non_empty_trait_info(self,
+                                                           g_mock,
+                                                           requests_mock):
+        """Test that attributes are set"""
+        mock_dataset = mock.MagicMock()
+        requests_mock.return_value = TestResponse()
+        type(mock_dataset).display_fields = mock.PropertyMock(
+            return_value=["a", "b", "c", "d"])
+        test_trait = retrieve_trait_info(trait=MockTrait(dataset=mock_dataset),
+                                         dataset=mock_dataset)
+        self.assertEqual(test_trait.a, 1)
+        self.assertEqual(test_trait.b, 2)
+        self.assertEqual(test_trait.c, 3)
+        self.assertEqual(test_trait.d, 4)
+
+    @mock.patch('base.trait.requests.get')
+    @mock.patch('base.trait.g')
+    def test_retrieve_trait_info_utf8_parsing(self,
+                                              g_mock,
+                                              requests_mock):
+        """Test that utf-8 strings are parsed correctly"""
+        utf_8_string = "test_string"
+        mock_dataset = mock.MagicMock()
+        requests_mock.return_value = TestResponse()
+        type(mock_dataset).display_fields = mock.PropertyMock(
+            return_value=["a", "b", "c", "d"])
+        type(mock_dataset).type = 'Publish'
+
+        mock_trait = MockTrait(
+            dataset=mock_dataset,
+            pre_publication_description=utf_8_string
+        )
+        trait_attrs = {
+            "group_code": "test_code",
+            "pre_publication_description": "test_pre_pub",
+            "pre_publication_abbreviation": "ファイルを画面毎に見て行くには、次のコマンドを使います。",
+            "post_publication_description": None,
+            "pubmed_id": None,
+            'year': "2020",
+            "authors": "Jane Doe かいと",
+        }
+        for key, val in list(trait_attrs.items()):
+            setattr(mock_trait, key, val)
+        test_trait = retrieve_trait_info(trait=mock_trait,
+                                         dataset=mock_dataset)
+        self.assertEqual(test_trait.abbreviation,
+                         "ファイルを画面毎に見て行くには、次のコマンドを使います。")
+        self.assertEqual(test_trait.authors,
+                         "Jane Doe かいと")
diff --git a/wqflask/tests/utility/test_authentication_tools.py b/wqflask/tests/utility/test_authentication_tools.py
new file mode 100644
index 00000000..99c74245
--- /dev/null
+++ b/wqflask/tests/utility/test_authentication_tools.py
@@ -0,0 +1,193 @@
+"""Tests for authentication tools"""
+import unittest
+import mock
+
+from utility.authentication_tools import check_resource_availability
+from utility.authentication_tools import add_new_resource
+
+
+class TestResponse:
+    """Mock Test Response after a request"""
+    @property
+    def content(self):
+        """Mock the content from Requests.get(params).content"""
+        return '["foo"]'
+
+
+class TestUser:
+    """Mock user"""
+    @property
+    def user_id(self):
+        """Mockes user id. Used in Flask.g.user_session.user_id"""
+        return "Jane"
+
+
+class TestUserSession:
+    """Mock user session"""
+    @property
+    def user_session(self):
+        """Mock user session. Mocks Flask.g.user_session object"""
+        return TestUser()
+
+
+def mock_add_resource(resource_ob, update=False):
+    return resource_ob
+
+
+class TestCheckResourceAvailability(unittest.TestCase):
+    """Test methods related to checking the resource availability"""
+    @mock.patch('utility.authentication_tools.add_new_resource')
+    @mock.patch('utility.authentication_tools.Redis')
+    @mock.patch('utility.authentication_tools.g')
+    @mock.patch('utility.authentication_tools.get_resource_id')
+    def test_check_resource_availability_default_mask(
+            self,
+            resource_id_mock,
+            g_mock,
+            redis_mock,
+            add_new_resource_mock):
+        """Test the resource availability with default mask"""
+        resource_id_mock.return_value = 1
+        g_mock.return_value = mock.Mock()
+        redis_mock.smembers.return_value = []
+        test_dataset = mock.MagicMock()
+        type(test_dataset).type = mock.PropertyMock(return_value="Test")
+        add_new_resource_mock.return_value = {"default_mask": 2}
+        self.assertEqual(check_resource_availability(test_dataset), 2)
+
+    @mock.patch('utility.authentication_tools.requests.get')
+    @mock.patch('utility.authentication_tools.add_new_resource')
+    @mock.patch('utility.authentication_tools.Redis')
+    @mock.patch('utility.authentication_tools.g')
+    @mock.patch('utility.authentication_tools.get_resource_id')
+    def test_check_resource_availability_non_default_mask(
+            self,
+            resource_id_mock,
+            g_mock,
+            redis_mock,
+            add_new_resource_mock,
+            requests_mock):
+        """Test the resource availability with default mask"""
+        resource_id_mock.return_value = 1
+        g_mock.return_value = mock.Mock()
+        redis_mock.smembers.return_value = []
+        add_new_resource_mock.return_value = {"default_mask": 2}
+        requests_mock.return_value = TestResponse()
+        test_dataset = mock.MagicMock()
+        type(test_dataset).type = mock.PropertyMock(return_value="Test")
+        self.assertEqual(check_resource_availability(test_dataset),
+                         ['foo'])
+
+    @mock.patch('utility.authentication_tools.webqtlConfig.SUPER_PRIVILEGES',
+                "SUPERUSER")
+    @mock.patch('utility.authentication_tools.requests.get')
+    @mock.patch('utility.authentication_tools.add_new_resource')
+    @mock.patch('utility.authentication_tools.Redis')
+    @mock.patch('utility.authentication_tools.g', TestUserSession())
+    @mock.patch('utility.authentication_tools.get_resource_id')
+    def test_check_resource_availability_of_super_user(
+            self,
+            resource_id_mock,
+            redis_mock,
+            add_new_resource_mock,
+            requests_mock):
+        """Test the resource availability if the user is the super user"""
+        resource_id_mock.return_value = 1
+        redis_mock.smembers.return_value = ["Jane"]
+        add_new_resource_mock.return_value = {"default_mask": 2}
+        requests_mock.return_value = TestResponse()
+        test_dataset = mock.MagicMock()
+        type(test_dataset).type = mock.PropertyMock(return_value="Test")
+        self.assertEqual(check_resource_availability(test_dataset),
+                         "SUPERUSER")
+
+    @mock.patch('utility.authentication_tools.webqtlConfig.DEFAULT_PRIVILEGES',
+                "John Doe")
+    def test_check_resource_availability_string_dataset(self):
+        """Test the resource availability if the dataset is a string"""
+        self.assertEqual(check_resource_availability("Test"),
+                         "John Doe")
+
+    @mock.patch('utility.authentication_tools.webqtlConfig.DEFAULT_PRIVILEGES',
+                "John Doe")
+    def test_check_resource_availability_temp(self):
+        """Test the resource availability if the dataset is a string"""
+        test_dataset = mock.MagicMock()
+        type(test_dataset).type = mock.PropertyMock(return_value="Temp")
+        self.assertEqual(check_resource_availability(test_dataset),
+                         "John Doe")
+
+
+class TestAddNewResource(unittest.TestCase):
+    """Test cases for add_new_resource method"""
+    @mock.patch('utility.authentication_tools.webqtlConfig.DEFAULT_PRIVILEGES',
+                "John Doe")
+    @mock.patch('utility.authentication_tools.add_resource', mock_add_resource)
+    @mock.patch('utility.authentication_tools.get_group_code')
+    def test_add_new_resource_if_publish_datatype(self, group_code_mock):
+        """Test add_new_resource if dataset type is 'publish'"""
+        group_code_mock.return_value = "Test"
+        test_dataset = mock.MagicMock()
+        type(test_dataset).type = mock.PropertyMock(return_value="Publish")
+        type(test_dataset).id = mock.PropertyMock(return_value=10)
+        expected_value = {
+            "owner_id": "none",
+            "default_mask": "John Doe",
+            "group_masks": {},
+            "name": "Test_None",
+            "data": {
+                "dataset": 10,
+                "trait": None
+            },
+            "type": "dataset-publish"
+        }
+        self.assertEqual(add_new_resource(test_dataset),
+                         expected_value)
+
+    @mock.patch('utility.authentication_tools.webqtlConfig.DEFAULT_PRIVILEGES',
+                "John Doe")
+    @mock.patch('utility.authentication_tools.add_resource', mock_add_resource)
+    @mock.patch('utility.authentication_tools.get_group_code')
+    def test_add_new_resource_if_geno_datatype(self, group_code_mock):
+        """Test add_new_resource if dataset type is 'geno'"""
+        group_code_mock.return_value = "Test"
+        test_dataset = mock.MagicMock()
+        type(test_dataset).name = mock.PropertyMock(return_value="Geno")
+        type(test_dataset).type = mock.PropertyMock(return_value="Geno")
+        type(test_dataset).id = mock.PropertyMock(return_value=20)
+        expected_value = {
+            "owner_id": "none",
+            "default_mask": "John Doe",
+            "group_masks": {},
+            "name": "Geno",
+            "data": {
+                "dataset": 20,
+            },
+            "type": "dataset-geno"
+        }
+        self.assertEqual(add_new_resource(test_dataset),
+                         expected_value)
+
+    @mock.patch('utility.authentication_tools.webqtlConfig.DEFAULT_PRIVILEGES',
+                "John Doe")
+    @mock.patch('utility.authentication_tools.add_resource', mock_add_resource)
+    @mock.patch('utility.authentication_tools.get_group_code')
+    def test_add_new_resource_if_other_datatype(self, group_code_mock):
+        """Test add_new_resource if dataset type is not 'geno' or 'publish'"""
+        group_code_mock.return_value = "Test"
+        test_dataset = mock.MagicMock()
+        type(test_dataset).name = mock.PropertyMock(return_value="Geno")
+        type(test_dataset).type = mock.PropertyMock(return_value="other")
+        type(test_dataset).id = mock.PropertyMock(return_value=20)
+        expected_value = {
+            "owner_id": "none",
+            "default_mask": "John Doe",
+            "group_masks": {},
+            "name": "Geno",
+            "data": {
+                "dataset": 20,
+            },
+            "type": "dataset-probeset"
+        }
+        self.assertEqual(add_new_resource(test_dataset),
+                         expected_value)
diff --git a/wqflask/tests/utility/test_hmac.py b/wqflask/tests/utility/test_hmac.py
new file mode 100644
index 00000000..16b50771
--- /dev/null
+++ b/wqflask/tests/utility/test_hmac.py
@@ -0,0 +1,39 @@
+# -*- coding: utf-8 -*-
+"""Test hmac utility functions"""
+
+import unittest
+import mock
+
+from utility.hmac import data_hmac
+from utility.hmac import url_for_hmac
+from utility.hmac import hmac_creation
+
+
+class TestHmacUtil(unittest.TestCase):
+    """Test Utility method for hmac creation"""
+
+    @mock.patch("utility.hmac.app.config", {'SECRET_HMAC_CODE': "secret"})
+    def test_hmac_creation(self):
+        """Test hmac creation with a utf-8 string"""
+        self.assertEqual(hmac_creation("ファイ"), "7410466338cfe109e946")
+
+    @mock.patch("utility.hmac.app.config", {'SECRET_HMAC_CODE': "secret"})
+    def test_data_hmac(self):
+        """Test data_hmac fn with a utf-8 string"""
+        self.assertEqual(data_hmac("ファイ"), "ファイ:7410466338cfe109e946")
+
+    @mock.patch("utility.hmac.app.config", {'SECRET_HMAC_CODE': "secret"})
+    @mock.patch("utility.hmac.url_for")
+    def test_url_for_hmac_with_plain_url(self, mock_url):
+        """Test url_for_hmac without params"""
+        mock_url.return_value = "https://mock_url.com/ファイ/"
+        self.assertEqual(url_for_hmac("ファイ"),
+                         "https://mock_url.com/ファイ/?hm=05bc39e659b1948f41e7")
+
+    @mock.patch("utility.hmac.app.config", {'SECRET_HMAC_CODE': "secret"})
+    @mock.patch("utility.hmac.url_for")
+    def test_url_for_hmac_with_param_in_url(self, mock_url):
+        """Test url_for_hmac with params"""
+        mock_url.return_value = "https://mock_url.com/?ファイ=1"
+        self.assertEqual(url_for_hmac("ファイ"),
+                         "https://mock_url.com/?ファイ=1&hm=4709c1708270644aed79")
diff --git a/wqflask/utility/authentication_tools.py b/wqflask/utility/authentication_tools.py
index 3553b92b..239b08e3 100644
--- a/wqflask/utility/authentication_tools.py
+++ b/wqflask/utility/authentication_tools.py
@@ -1,4 +1,6 @@
 from __future__ import absolute_import, print_function, division
+import logging
+from flask import Flask, g, redirect, url_for
 
 import json
 import requests
@@ -9,33 +11,31 @@ from utility import hmac
 from utility.redis_tools import get_redis_conn, get_resource_info, get_resource_id, add_resource
 Redis = get_redis_conn()
 
-from flask import Flask, g, redirect, url_for
 
-import logging
-logger = logging.getLogger(__name__ )
+logger = logging.getLogger(__name__)
+
 
 def check_resource_availability(dataset, trait_id=None):
 
-    #At least for now assume temporary entered traits are accessible
-    if type(dataset) == str:
-        return webqtlConfig.DEFAULT_PRIVILEGES
-    if dataset.type == "Temp":
+    # At least for now assume temporary entered traits are accessible
+    if type(dataset) == str or dataset.type == "Temp":
         return webqtlConfig.DEFAULT_PRIVILEGES
 
     resource_id = get_resource_id(dataset, trait_id)
 
-    if resource_id: #ZS: This should never be false, but it's technically possible if a non-Temp dataset somehow had a type other than Publish/ProbeSet/Geno
+    if resource_id:  # ZS: This should never be false, but it's technically possible if a non-Temp dataset somehow had a type other than Publish/ProbeSet/Geno
         resource_info = get_resource_info(resource_id)
-        if not resource_info: #ZS: If resource isn't already in redis, add it with default privileges
+        if not resource_info:  # ZS: If resource isn't already in redis, add it with default privileges
             resource_info = add_new_resource(dataset, trait_id)
 
-    #ZS: Check if super-user - we should probably come up with some way to integrate this into the proxy
+    # ZS: Check if super-user - we should probably come up with some way to integrate this into the proxy
     if g.user_session.user_id in Redis.smembers("super_users"):
-       return webqtlConfig.SUPER_PRIVILEGES
+        return webqtlConfig.SUPER_PRIVILEGES
 
     response = None
 
-    the_url = "http://localhost:8080/available?resource={}&user={}".format(resource_id, g.user_session.user_id)
+    the_url = "http://localhost:8080/available?resource={}&user={}".format(
+        resource_id, g.user_session.user_id)
     try:
         response = json.loads(requests.get(the_url).content)
     except:
@@ -43,18 +43,19 @@ def check_resource_availability(dataset, trait_id=None):
 
     return response
 
+
 def add_new_resource(dataset, trait_id=None):
     resource_ob = {
-        'owner_id'    : "none", # webqtlConfig.DEFAULT_OWNER_ID,
+        'owner_id': "none",  # webqtlConfig.DEFAULT_OWNER_ID,
         'default_mask': webqtlConfig.DEFAULT_PRIVILEGES,
-        'group_masks' : {}
+        'group_masks': {}
     }
 
     if dataset.type == "Publish":
         resource_ob['name'] = get_group_code(dataset) + "_" + str(trait_id)
         resource_ob['data'] = {
             'dataset': dataset.id,
-            'trait'  : trait_id
+            'trait': trait_id
         }
         resource_ob['type'] = 'dataset-publish'
     elif dataset.type == "Geno":
@@ -74,15 +75,19 @@ def add_new_resource(dataset, trait_id=None):
 
     return resource_info
 
+
 def get_group_code(dataset):
-    results = g.db.execute("SELECT InbredSetCode from InbredSet where Name='{}'".format(dataset.group.name)).fetchone()
+    results = g.db.execute("SELECT InbredSetCode from InbredSet where Name='{}'".format(
+        dataset.group.name)).fetchone()
     if results[0]:
         return results[0]
     else:
         return ""
 
+
 def check_admin(resource_id=None):
-    the_url = "http://localhost:8080/available?resource={}&user={}".format(resource_id, g.user_session.user_id)
+    the_url = "http://localhost:8080/available?resource={}&user={}".format(
+        resource_id, g.user_session.user_id)
     try:
         response = json.loads(requests.get(the_url).content)['admin']
     except:
@@ -96,6 +101,7 @@ def check_admin(resource_id=None):
     else:
         return "not-admin"
 
+
 def check_owner(dataset=None, trait_id=None, resource_id=None):
     if resource_id:
         resource_info = get_resource_info(resource_id)
@@ -110,6 +116,7 @@ def check_owner(dataset=None, trait_id=None, resource_id=None):
 
     return False
 
+
 def check_owner_or_admin(dataset=None, trait_id=None, resource_id=None):
     if not resource_id:
         if dataset.type == "Temp":
diff --git a/wqflask/utility/hmac.py b/wqflask/utility/hmac.py
index b08be97e..fd75803e 100644
--- a/wqflask/utility/hmac.py
+++ b/wqflask/utility/hmac.py
@@ -7,11 +7,11 @@ from flask import url_for
 
 from wqflask import app
 
+
 def hmac_creation(stringy):
     """Helper function to create the actual hmac"""
 
     secret = app.config['SECRET_HMAC_CODE']
-
     hmaced = hmac.new(secret, stringy, hashlib.sha1)
     hm = hmaced.hexdigest()
     # ZS: Leaving the below comment here to ask Pjotr about
@@ -20,10 +20,12 @@ def hmac_creation(stringy):
     hm = hm[:20]
     return hm
 
+
 def data_hmac(stringy):
-    """Takes arbitray data string and appends :hmac so we know data hasn't been tampered with"""
+    """Takes arbitrary data string and appends :hmac so we know data hasn't been tampered with"""
     return stringy + ":" + hmac_creation(stringy)
 
+
 def url_for_hmac(endpoint, **values):
     """Like url_for but adds an hmac at the end to insure the url hasn't been tampered with"""
 
@@ -36,5 +38,6 @@ def url_for_hmac(endpoint, **values):
         combiner = "?"
     return url + combiner + "hm=" + hm
 
+
 app.jinja_env.globals.update(url_for_hmac=url_for_hmac,
-                             data_hmac=data_hmac)
\ No newline at end of file
+                             data_hmac=data_hmac)
diff --git a/wqflask/utility/redis_tools.py b/wqflask/utility/redis_tools.py
index 81ba04ea..ef02268e 100644
--- a/wqflask/utility/redis_tools.py
+++ b/wqflask/utility/redis_tools.py
@@ -4,23 +4,21 @@ import uuid
 import simplejson as json
 import datetime
 
-import redis # used for collections
-
-import logging
-
-from flask import (render_template, flash)
-
-from utility import hmac
+import redis  # used for collections
 
+from utility.hmac import hmac_creation
 from utility.logger import getLogger
 logger = getLogger(__name__)
 
+
 def get_redis_conn():
     Redis = redis.StrictRedis(port=6379)
     return Redis
 
+
 Redis = get_redis_conn()
 
+
 def is_redis_available():
     try:
         Redis.ping()
@@ -28,6 +26,7 @@ def is_redis_available():
         return False
     return True
 
+
 def get_user_id(column_name, column_value):
     user_list = Redis.hgetall("users")
     key_list = []
@@ -38,6 +37,7 @@ def get_user_id(column_name, column_value):
 
     return None
 
+
 def get_user_by_unique_column(column_name, column_value):
     item_details = None
 
@@ -52,9 +52,11 @@ def get_user_by_unique_column(column_name, column_value):
 
     return item_details
 
+
 def get_users_like_unique_column(column_name, column_value):
-    """
-    Like previous function, but this only checks if the input is a subset of a field and can return multiple results
+    """Like previous function, but this only checks if the input is a
+    subset of a field and can return multiple results
+
     """
     matched_users = []
 
@@ -74,7 +76,6 @@ def get_users_like_unique_column(column_name, column_value):
 
     return matched_users
 
-# def search_users_by_unique_column(column_name, column_value):
 
 def set_user_attribute(user_id, column_name, column_value):
     user_info = json.loads(Redis.hget("users", user_id))
@@ -82,6 +83,7 @@ def set_user_attribute(user_id, column_name, column_value):
 
     Redis.hset("users", user_id, json.dumps(user_info))
 
+
 def get_user_collections(user_id):
     collections = None
     collections = Redis.hget("collections", user_id)
@@ -91,22 +93,27 @@ def get_user_collections(user_id):
     else:
         return []
 
+
 def save_user(user, user_id):
     Redis.hset("users", user_id, json.dumps(user))
 
+
 def save_collections(user_id, collections_ob):
     Redis.hset("collections", user_id, collections_ob)
 
+
 def save_verification_code(user_email, code):
     Redis.hset("verification_codes", code, user_email)
 
+
 def check_verification_code(code):
     email_address = None
     user_details = None
     email_address = Redis.hget("verification_codes", code)
 
     if email_address:
-        user_details = get_user_by_unique_column('email_address', email_address)
+        user_details = get_user_by_unique_column(
+            'email_address', email_address)
         if user_details:
             return user_details
         else:
@@ -114,10 +121,12 @@ def check_verification_code(code):
     else:
         return None
 
+
 def get_user_groups(user_id):
-    #ZS: Get the groups where a user is an admin or a member and return lists corresponding to those two sets of groups
-    admin_group_ids = []  #ZS: Group IDs where user is an admin
-    user_group_ids = []   #ZS: Group IDs where user is a regular user
+    # ZS: Get the groups where a user is an admin or a member and
+    # return lists corresponding to those two sets of groups
+    admin_group_ids = []  # ZS: Group IDs where user is an admin
+    user_group_ids = []  # ZS: Group IDs where user is a regular user
     groups_list = Redis.hgetall("groups")
     for key in groups_list:
         try:
@@ -142,6 +151,7 @@ def get_user_groups(user_id):
 
     return admin_groups, user_groups
 
+
 def get_group_info(group_id):
     group_json = Redis.hget("groups", group_id)
     group_info = None
@@ -150,6 +160,7 @@ def get_group_info(group_id):
 
     return group_info
 
+
 def get_group_by_unique_column(column_name, column_value):
     """ Get group by column; not sure if there's a faster way to do this """
 
@@ -158,7 +169,8 @@ def get_group_by_unique_column(column_name, column_value):
     all_group_list = Redis.hgetall("groups")
     for key in all_group_list:
         group_info = json.loads(all_group_list[key])
-        if column_name == "admins" or column_name == "members": #ZS: Since these fields are lists, search in the list
+        # ZS: Since these fields are lists, search in the list
+        if column_name == "admins" or column_name == "members":
             if column_value in group_info[column_name]:
                 matched_groups.append(group_info)
         else:
@@ -167,9 +179,11 @@ def get_group_by_unique_column(column_name, column_value):
 
     return matched_groups
 
+
 def get_groups_like_unique_column(column_name, column_value):
-    """
-    Like previous function, but this only checks if the input is a subset of a field and can return multiple results
+    """Like previous function, but this only checks if the input is a
+    subset of a field and can return multiple results
+
     """
     matched_groups = []
 
@@ -178,7 +192,8 @@ def get_groups_like_unique_column(column_name, column_value):
         if column_name != "group_id":
             for key in group_list:
                 group_info = json.loads(group_list[key])
-                if column_name == "admins" or column_name == "members": #ZS: Since these fields are lists, search in the list
+                # ZS: Since these fields are lists, search in the list
+                if column_name == "admins" or column_name == "members":
                     if column_value in group_info[column_name]:
                         matched_groups.append(group_info)
                 else:
@@ -190,13 +205,15 @@ def get_groups_like_unique_column(column_name, column_value):
 
     return matched_groups
 
-def create_group(admin_user_ids, member_user_ids = [], group_name = "Default Group Name"):
+
+def create_group(admin_user_ids, member_user_ids=[],
+                 group_name="Default Group Name"):
     group_id = str(uuid.uuid4())
     new_group = {
-        "id"    : group_id,
+        "id": group_id,
         "admins": admin_user_ids,
-        "members" : member_user_ids,
-        "name"  : group_name,
+        "members": member_user_ids,
+        "name": group_name,
         "created_timestamp": datetime.datetime.utcnow().strftime('%b %d %Y %I:%M%p'),
         "changed_timestamp": datetime.datetime.utcnow().strftime('%b %d %Y %I:%M%p')
     }
@@ -205,8 +222,9 @@ def create_group(admin_user_ids, member_user_ids = [], group_name = "Default Gro
 
     return new_group
 
+
 def delete_group(user_id, group_id):
-    #ZS: If user is an admin of a group, remove it from the groups hash
+    # ZS: If user is an admin of a group, remove it from the groups hash
     group_info = get_group_info(group_id)
     if user_id in group_info["admins"]:
         Redis.hdel("groups", group_id)
@@ -214,9 +232,15 @@ def delete_group(user_id, group_id):
     else:
         None
 
-def add_users_to_group(user_id, group_id, user_emails = [], admins = False): #ZS "admins" is just to indicate whether the users should be added to the groups admins or regular users set
+
+# ZS "admins" is just to indicate whether the users should be added to
+# the groups admins or regular users set
+def add_users_to_group(user_id, group_id, user_emails=[], admins=False):
     group_info = get_group_info(group_id)
-    if user_id in group_info["admins"]: #ZS: Just to make sure that the user is an admin for the group, even though they shouldn't be able to reach this point unless they are
+    # ZS: Just to make sure that the user is an admin for the group,
+    # even though they shouldn't be able to reach this point unless
+    # they are
+    if user_id in group_info["admins"]:
         if admins:
             group_users = set(group_info["admins"])
         else:
@@ -231,25 +255,36 @@ def add_users_to_group(user_id, group_id, user_emails = [], admins = False): #ZS
         else:
             group_info["members"] = list(group_users)
 
-        group_info["changed_timestamp"] = datetime.datetime.utcnow().strftime('%b %d %Y %I:%M%p')
+        group_info["changed_timestamp"] = datetime.datetime.utcnow().strftime(
+            '%b %d %Y %I:%M%p')
         Redis.hset("groups", group_id, json.dumps(group_info))
         return group_info
     else:
         return None
 
-def remove_users_from_group(user_id, users_to_remove_ids, group_id, user_type = "members"): #ZS: User type is because I assume admins can remove other admins
+
+# ZS: User type is because I assume admins can remove other admins
+def remove_users_from_group(user_id,
+                            users_to_remove_ids,
+                            group_id,
+                            user_type="members"):
     group_info = get_group_info(group_id)
 
     if user_id in group_info["admins"]:
         users_to_remove_set = set(users_to_remove_ids)
-        if user_type == "admins" and user_id in users_to_remove_set: #ZS: Make sure an admin can't remove themselves from a group, since I imagine we don't want groups to be able to become admin-less
+        # ZS: Make sure an admin can't remove themselves from a group,
+        # since I imagine we don't want groups to be able to become
+        # admin-less
+        if user_type == "admins" and user_id in users_to_remove_set:
             users_to_remove_set.remove(user_id)
         group_users = set(group_info[user_type])
         group_users -= users_to_remove_set
         group_info[user_type] = list(group_users)
-        group_info["changed_timestamp"] = datetime.datetime.utcnow().strftime('%b %d %Y %I:%M%p')
+        group_info["changed_timestamp"] = datetime.datetime.utcnow().strftime(
+            '%b %d %Y %I:%M%p')
         Redis.hset("groups", group_id, json.dumps(group_info))
 
+
 def change_group_name(user_id, group_id, new_name):
     group_info = get_group_info(group_id)
     if user_id in group_info["admins"]:
@@ -259,22 +294,28 @@ def change_group_name(user_id, group_id, new_name):
     else:
         return None
 
+
 def get_resources():
     resource_list = Redis.hgetall("resources")
     return resource_list
 
+
 def get_resource_id(dataset, trait_id=None):
     resource_id = False
     if dataset.type == "Publish":
         if trait_id:
-            resource_id = hmac.hmac_creation("{}:{}:{}".format('dataset-publish', dataset.id, trait_id))
+            resource_id = hmac_creation("{}:{}:{}".format(
+                'dataset-publish', dataset.id, trait_id))
     elif dataset.type == "ProbeSet":
-        resource_id = hmac.hmac_creation("{}:{}".format('dataset-probeset', dataset.id))
+        resource_id = hmac_creation(
+            "{}:{}".format('dataset-probeset', dataset.id))
     elif dataset.type == "Geno":
-        resource_id = hmac.hmac_creation("{}:{}".format('dataset-geno', dataset.id))
+        resource_id = hmac_creation(
+            "{}:{}".format('dataset-geno', dataset.id))
 
     return resource_id
 
+
 def get_resource_info(resource_id):
     resource_info = Redis.hget("resources", resource_id)
     if resource_info:
@@ -282,17 +323,23 @@ def get_resource_info(resource_id):
     else:
         return None
 
+
 def add_resource(resource_info, update=True):
     if 'trait' in resource_info['data']:
-        resource_id = hmac.hmac_creation('{}:{}:{}'.format(str(resource_info['type']), str(resource_info['data']['dataset']), str(resource_info['data']['trait'])))
+        resource_id = hmac_creation('{}:{}:{}'.format(
+            str(resource_info['type']), str(
+                resource_info['data']['dataset']),
+            str(resource_info['data']['trait'])))
     else:
-        resource_id = hmac.hmac_creation('{}:{}'.format(str(resource_info['type']), str(resource_info['data']['dataset'])))
+        resource_id = hmac_creation('{}:{}'.format(
+            str(resource_info['type']), str(resource_info['data']['dataset'])))
 
     if update or not Redis.hexists("resources", resource_id):
         Redis.hset("resources", resource_id, json.dumps(resource_info))
 
     return resource_info
 
+
 def add_access_mask(resource_id, group_id, access_mask):
     the_resource = get_resource_info(resource_id)
     the_resource['group_masks'][group_id] = access_mask
@@ -301,9 +348,10 @@ def add_access_mask(resource_id, group_id, access_mask):
 
     return the_resource
 
+
 def change_resource_owner(resource_id, new_owner_id):
-    the_resource= get_resource_info(resource_id)
+    the_resource = get_resource_info(resource_id)
     the_resource['owner_id'] = new_owner_id
 
     Redis.delete("resource")
-    Redis.hset("resources", resource_id, json.dumps(the_resource))
\ No newline at end of file
+    Redis.hset("resources", resource_id, json.dumps(the_resource))