Skip to content

Commit

Permalink
update: fix tests, fix init script, implement bulk actions
Browse files Browse the repository at this point in the history
  • Loading branch information
mutantsan committed Jan 2, 2024
1 parent 3fcd89a commit 414c6bf
Show file tree
Hide file tree
Showing 11 changed files with 154 additions and 91 deletions.
25 changes: 17 additions & 8 deletions ckanext/tour/assets/js/tour-init.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ this.ckan.module('tour-init', function (jQuery) {
return md.mobile() ? true : false;
},

/**
* Creates a tour mark if not exist
*
* @returns
*/
createMark: function () {
if (!this.mark) {
this.mark = jQuery(this.options.template);
Expand All @@ -39,8 +44,10 @@ this.ckan.module('tour-init', function (jQuery) {
},

_initIntro: function (introData) {
console.log(introData);
var showed = localStorage.getItem('intro-' + introData.id);
var shouldStart = !showed && !this.isMobile;
var shouldAttach = introData.state === "active";
var shouldStart = !showed && !this.isMobile && window.location.pathname == introData.page;
var anchorExists = $(introData.anchor).length;

this.intro = introJs();
Expand All @@ -60,15 +67,17 @@ this.ckan.module('tour-init', function (jQuery) {
steps: this._prepareSteps(introData.steps),
});

if (shouldAttach && !shouldStart) {
this.createMark();

this.createMark().appendTo(anchorExists ? introData.anchor : '.breadcrumb .active');
this.mark.on('click', this._onClick);
this.mark.insertAfter(anchorExists ? introData.anchor : '.breadcrumb .active');
this.mark.on('click', this._onClick);
}

// if (shouldStart) {
// // for development
// localStorage.setItem('intro-' + introData.id, 1);
// this.intro.start();
// }
if (shouldStart) {
localStorage.setItem('intro-' + introData.id, 1);
this.intro.start();
}
},

_prepareSteps: function (steps) {
Expand Down
49 changes: 22 additions & 27 deletions ckanext/tour/logic/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
from ckan.logic import validate

import ckanext.tour.logic.schema as schema
import ckanext.tour.model as tour_model
from ckanext.tour.exception import TourStepFileError
from ckanext.tour.model import Tour, TourStep, TourStepImage


Expand Down Expand Up @@ -47,10 +45,18 @@ def tour_create(context, data_dict):
for step in steps:
step["tour_id"] = tour.id

tk.get_action("tour_step_create")(
{"ignore_auth": True},
step,
)
try:
tk.get_action("tour_step_create")(
{"ignore_auth": True},
step,
)
except tk.ValidationError as e:
tk.get_action("tour_remove")(
{"ignore_auth": True},
{"id": tour.id},
)

raise tk.ValidationError(e.error_dict if e else {})

return tour.dictize(context)

Expand Down Expand Up @@ -94,7 +100,7 @@ def tour_step_create(context, data_dict):
"tour_step_id": tour_step.id,
},
)
except TourStepFileError as e:
except tk.ValidationError as e:
raise tk.ValidationError(
{"image": [f"Error while uploading step image: {e}"]}
)
Expand All @@ -108,9 +114,10 @@ def tour_update(context, data_dict):

tour = cast(Tour, Tour.get(data_dict["id"]))

tour.title = data_dict["title"]
tour.anchor = data_dict["anchor"]
tour.page = data_dict["page"]
tour.title = data_dict.get("title", tour.title)
tour.anchor = data_dict.get("anchor", tour.anchor)
tour.page = data_dict.get("page", tour.page)
tour.state = data_dict.get("state", tour.page)

model.Session.commit()

Expand Down Expand Up @@ -161,7 +168,7 @@ def tour_step_update(context, data_dict):

try:
tk.get_action(action)({"ignore_auth": True}, data_dict["image"][0])
except TourStepFileError as e:
except tk.ValidationError as e:
raise tk.ValidationError(
{"image": [f"Error while uploading step image: {e}"]}
)
Expand All @@ -186,12 +193,7 @@ def tour_step_image_upload(context, data_dict):
tour_step_id = data_dict.pop("tour_step_id", None)

if not any([data_dict.get("upload"), data_dict.get("url")]):
raise TourStepFileError(tk._("You have to provide either file or URL"))

if all([data_dict.get("upload"), data_dict.get("url")]):
raise TourStepFileError(
tk._("You cannot use a file and a URL at the same time")
)
raise tk.ValidationError(tk._("You have to provide either file or URL"))

if not data_dict.get("upload"):
return TourStepImage.create(
Expand All @@ -206,10 +208,8 @@ def tour_step_image_upload(context, data_dict):
"upload": data_dict["upload"],
},
)
except tk.ValidationError as e:
raise TourStepFileError(e.error_summary)
except OSError as e:
raise TourStepFileError(e)
raise tk.ValidationError(str(e))

data_dict["file_id"] = result["id"]

Expand All @@ -223,12 +223,7 @@ def tour_step_image_update(context, data_dict):
tk.check_access("tour_step_update", context, data_dict)

if not any([data_dict.get("upload"), data_dict.get("url")]):
raise TourStepFileError(tk._("You have to provide either file or URL"))

if all([data_dict.get("upload"), data_dict.get("url")]):
raise TourStepFileError(
tk._("You cannot use a file and a URL at the same time")
)
raise tk.ValidationError(tk._("You have to provide either file or URL"))

tour_step_image = cast(
TourStepImage,
Expand All @@ -249,7 +244,7 @@ def tour_step_image_update(context, data_dict):
},
)
except (tk.ValidationError, OSError) as e:
raise TourStepFileError(str(e))
raise tk.ValidationError(str(e))

tour_step_image.url = result["url"]

Expand Down
7 changes: 5 additions & 2 deletions ckanext/tour/logic/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from typing import Any, Dict

from ckan.lib.navl.validators import ignore_empty
from ckan.logic.schema import validator_args

from ckanext.tour.model import Tour, TourStep
Expand Down Expand Up @@ -47,11 +46,15 @@ def tour_update(
not_empty,
unicode_safe,
tour_tour_exist,
ignore_empty
) -> Schema:
tour_schema = tour_create()
tour_schema["id"] = [not_empty, unicode_safe, tour_tour_exist]
tour_schema["steps"] = tour_step_update()

# non-mandatory
tour_schema["title"] = [ignore_empty, unicode_safe]

# we shouldn't be able to change an author_id
tour_schema.pop("author_id")

Expand Down Expand Up @@ -111,7 +114,7 @@ def tour_step_image_schema(
) -> Schema:
return {
"upload": [ignore_empty],
"url": [ignore_empty, unicode_safe, tour_url_validator],
"url": [ignore_empty, unicode_safe],
"tour_step_id": [not_missing, unicode_safe, tour_tour_step_exist],
}

Expand Down
10 changes: 10 additions & 0 deletions ckanext/tour/logic/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ def tour_url_validator(
if not url:
return

# step_idx = key[1]
# try:
# image_idx = key[3]
# except IndexError:
# import ipdb; ipdb.set_trace()
# pass

# if data.get(('steps', step_idx, 'image', image_idx, 'upload')):
# return

try:
pieces = urlparse(url)
if (
Expand Down
28 changes: 2 additions & 26 deletions ckanext/tour/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pytest_factoryboy import register

import ckanext.tour.tests.factories as tour_factories
from ckanext.tour.tests.helpers import CSV_DATA, FakeFileStorage
from ckanext.tour.tests.helpers import IMAGE_DATA, FakeFileStorage

fake = Faker()

Expand Down Expand Up @@ -43,35 +43,11 @@ def mock_storage(monkeypatch, ckan_config, tmpdir):
monkeypatch.setattr(uploader, "get_storage_path", lambda: str(tmpdir))


# @pytest.fixture
# def rd_study_data(study):
# def _prepare_data(**kwargs):
# request_study_id = fake.uuid4()

# data = {
# "id": request_study_id,
# "title": fake.sentence(),
# "use_main_request": True,
# "use_main_docs": False,
# "request_protocol": False,
# "request_raw": False,
# "request_clean": False,
# "request_shareable": False,
# "package_id": study["id"],
# "files": [],
# }
# data.update(**kwargs)

# return data

# yield _prepare_data


@pytest.fixture
def tour_image_data():
def _prepare_data(**kwargs):
data = {
"upload": FakeFileStorage(BytesIO(CSV_DATA), "data.csv"),
"upload": FakeFileStorage(BytesIO(IMAGE_DATA), "step.jpeg"),
"url": None,
}

Expand Down
6 changes: 3 additions & 3 deletions ckanext/tour/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from ckan.tests import factories

from ckanext.tour import model as tour_model
from ckanext.tour.tests.helpers import CSV_DATA, FakeFileStorage
from ckanext.tour.tests.helpers import IMAGE_DATA, FakeFileStorage


class TourStepImageFactory(factories.CKANFactory):
Expand All @@ -13,9 +13,9 @@ class Meta:
action = "tour_step_image_upload"

tour_step_id = factory.LazyFunction(lambda: TourStepFactory()["id"])
url = None
url = "step.jpeg"
upload = factory.LazyAttribute(
lambda _: FakeFileStorage(BytesIO(CSV_DATA), "step.jpeg")
lambda _: FakeFileStorage(BytesIO(IMAGE_DATA), "step.jpeg")
)


Expand Down
2 changes: 1 addition & 1 deletion ckanext/tour/tests/helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from werkzeug.datastructures import FileStorage as MockFileStorage # noqa

CSV_DATA = b"a,b,c,d\n1,2,3,4"
IMAGE_DATA = b"a,b,c,d\n1,2,3,4"


class FakeFileStorage(MockFileStorage):
Expand Down
66 changes: 47 additions & 19 deletions ckanext/tour/tests/logic/test_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,45 @@ def test_missing_element(self, tour_factory, tour_step_factory, tour_image_data)
with pytest.raises(tk.ValidationError, match="Missing value"):
tour_step_factory(tour_id=tour["id"], element=None)

def test_error_on_child_should_clear_parent(self, sysadmin):
"""When we are creating from the UI, we are passing all the tour data at
once and if something is wrong, do not create anything.
TODO: currently I wasn't able to check if something is wrong with Image data"""

with pytest.raises(tk.ValidationError):
call_action(
"tour_create",
title="test tour",
anchor="#page",
page="/datasets/",
steps=[
{
"title": "step #1",
"element": ".header",
"intro": "test intro",
}
],
)

assert not tour_model.Tour.all()

call_action(
"tour_create",
title="test tour",
anchor="#page",
page="/datasets/",
author_id=sysadmin["id"],
steps=[
{
"title": "step #1",
"element": ".header",
"intro": "test intro",
}
],
)

assert tour_model.Tour.all()

@pytest.mark.usefixtures("with_plugins", "clean_db", "mock_storage")
class TestTourUpdate:
Expand Down Expand Up @@ -134,7 +173,7 @@ class TestTourStepUpdate:
def test_update_not_existing(self):
with pytest.raises(
tk.ValidationError,
match="The tour with an id xxx doesn't exist",
match="The tour step with an id xxx doesn't exist",
):
call_action("tour_step_update", id="xxx")

Expand Down Expand Up @@ -174,17 +213,15 @@ class TestStepImageCreate:
"""Each step could have 1 image. It could be created either from uploaded file,
or by URL"""

def test_create_from_invalid_url(self, tour_step, tour_step_image_factory):
"""You should be able to use only url-like string for a URL field"""
with pytest.raises(tk.ValidationError, match="Please provide a valid URL"):
tour_step_image_factory(
tour_step_id=tour_step["id"], url="xxx", upload=None
)

def test_create_from_valid_url(self, tour_step, tour_step_image_factory):
def test_create_from_url(self, tour_step, tour_step_image_factory):
"""You should be able to create a step image entity from a URL.
We are not checking that this URL somehow related to an image, it's up
to user"""
to user
We are not validating URL, because if user choose to upload a file,
the URL will be a filename which is obviosly not a valid URL.
"""
image_from_url = tour_step_image_factory(
tour_step_id=tour_step["id"], url="https://image.url", upload=None
)
Expand All @@ -206,12 +243,3 @@ def test_create_from_nothing(self, tour_step, tour_step_image_factory):
tk.ValidationError, match="You have to provide either file or URL"
):
tour_step_image_factory(tour_step_id=tour_step["id"], url=None, upload=None)

def test_create_from_both(self, tour_step, tour_step_image_factory):
"""Obviously, you should either use URL of file, not both at the same time"""
with pytest.raises(
tk.ValidationError, match="You cannot use a file and a URL at the same time"
):
tour_step_image_factory(
tour_step_id=tour_step["id"], url="https://image.url"
)
Loading

0 comments on commit 414c6bf

Please sign in to comment.