Unverified Commit c3664644 authored by Dannon's avatar Dannon Committed by GitHub
Browse files

Merge pull request #16400 from davelopez/23.1_fix_histories_count 

[23.1] Fix histories count
parents 5cd2469a 78567b54

lib/galaxy/managers/histories.py

+9 −0
Original line number Diff line number Diff line
@@ -403,6 +403,15 @@ class HistoryManager(sharable.SharableModelManager, deletable.PurgableManagerMix 


        return history



    def get_active_count(self, user: model.User) -> int:

        """Return the number of active histories for the given user."""

        user_active_histories_filter = [

            model.History.user_id == user.id,

            model.History.deleted == false(),

            model.History.archived == false(),

        ]

        return self.count(filters=user_active_histories_filter)





class HistoryStorageCleanerManager(StorageCleanerManager):

    def __init__(self, history_manager: HistoryManager):

lib/galaxy/model/__init__.py

+1 −1
Original line number Diff line number Diff line
@@ -702,7 +702,7 @@ class User(Base, Dictifiable, RepresentById): 
    )

    active_histories = relationship(

        "History",

        primaryjoin=(lambda: (History.user_id == User.id) & (not_(History.deleted))),  # type: ignore[has-type]

        primaryjoin=(lambda: (History.user_id == User.id) & (not_(History.deleted)) & (not_(History.archived))),  # type: ignore[has-type]

        viewonly=True,

        order_by=lambda: desc(History.update_time),  # type: ignore[has-type]

    )

lib/galaxy/webapps/galaxy/services/histories.py

+2 −4
Original line number Diff line number Diff line
@@ -476,10 +476,8 @@ class HistoriesService(ServiceBase, ConsumesModelStores, ServesExportStores): 
        current_user = self.user_manager.current_user(trans)

        if self.user_manager.is_anonymous(current_user):

            current_history = self.manager.get_current(trans)

            if not current_history:

                return 0

            return 1

        return len(current_user.active_histories)

            return 1 if current_history else 0

        return self.manager.get_active_count(current_user)



    def published(

        self,

lib/galaxy_test/api/test_histories.py

+30 −0
Original line number Diff line number Diff line
@@ -407,6 +407,36 @@ class TestHistoriesApi(ApiTestCase, BaseHistories): 
        response = self.dataset_populator.tag_dataset(history_id, hda["id"], ["DatasetTag"], raise_on_error=False)

        assert response["err_msg"] == "History is immutable"



    def test_histories_count(self):

        # Create a new user so we can test the count without other existing histories

        with self._different_user("user_for_count@test.com"):

            first_history_id = self._create_history("TestHistoryForCount 1")["id"]

            self._assert_expected_histories_count(expected_count=1)



            second_history_id = self._create_history("TestHistoryForCount 2")["id"]

            self._assert_expected_histories_count(expected_count=2)



            third_history_id = self._create_history("TestHistoryForCount 3")["id"]

            self._assert_expected_histories_count(expected_count=3)



            # Delete the second history

            self.dataset_populator.delete_history(second_history_id)

            self._assert_expected_histories_count(expected_count=2)



            # Archive the first history

            self.dataset_populator.archive_history(first_history_id)

            self._assert_expected_histories_count(expected_count=1)



            # Only the third history should be active

            active_histories = self._get("histories").json()

            assert len(active_histories) == 1

            assert active_histories[0]["id"] == third_history_id



    def _assert_expected_histories_count(self, expected_count):

        response = self._get("histories/count")

        self._assert_status_code_is(response, 200)

        assert response.json() == expected_count





class ImportExportTests(BaseHistories):

    task_based: ClassVar[bool]

test/unit/app/managers/test_HistoryManager.py

+15 −5
Original line number Diff line number Diff line
@@ -2,7 +2,10 @@ from unittest import mock 


import pytest

import sqlalchemy

from sqlalchemy import true

from sqlalchemy import (

    false,

    true,

)



from galaxy import (

    exceptions,
@@ -956,10 +959,12 @@ class TestHistoryFilters(BaseTestCase): 
        history2 = self.history_manager.create(name="history2", user=user2)

        history3 = self.history_manager.create(name="history3", user=user2)

        history4 = self.history_manager.create(name="history4", user=user2)

        history5 = self.history_manager.create(name="history5", user=user2)



        self.history_manager.delete(history1)

        self.history_manager.delete(history2)

        self.history_manager.delete(history3)

        self.history_manager.archive_history(history3, None)

        self.history_manager.archive_history(history4, None)



        test_annotation = "testing"

        history2.add_item_annotation(self.trans.sa_session, user2, history2, test_annotation)
@@ -969,12 +974,17 @@ class TestHistoryFilters(BaseTestCase): 
        history3.add_item_annotation(self.trans.sa_session, user2, history4, test_annotation)

        self.trans.sa_session.flush()



        all_histories = [history1, history2, history3, history4]

        deleted = [history1, history2, history3]

        all_histories = [history1, history2, history3, history4, history5]

        deleted = [history1, history2]

        archived = [history3, history4]



        assert self.history_manager.count() == len(all_histories), "having no filters should count all histories"

        filters = [model.History.deleted == true()]

        assert self.history_manager.count(filters=filters) == len(deleted), "counting with orm filters should work"

        assert self.history_manager.count(filters=filters) == len(deleted)

        filters = [model.History.archived == true()]

        assert self.history_manager.count(filters=filters) == len(archived)

        filters = [model.History.deleted == false(), model.History.archived == false()]

        assert self.history_manager.count(filters=filters) == len(all_histories) - len(deleted) - len(archived)



        raw_annotation_fn_filter = ("annotation", "has", test_annotation)

        # functional filtering is not supported