Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Queries UI #16392

Closed
wants to merge 4 commits into from
Closed

Fix Queries UI #16392

wants to merge 4 commits into from

Conversation

MikeAlhayek
Copy link
Member

Fixes #16372

/cc @gvkries

@gvkries
Copy link
Contributor

gvkries commented Jul 2, 2024

Your UI changes are good, even though it is not strictly necessary. We could keep the edit functionality of the base Query type, no data would be lost because of the extension property. But of cause it doesn't make sense to edit it.

But we loose every query of a deactivated feature, because they will only ever be deserialized as Query after changes to the underlying document.

@gvkries
Copy link
Contributor

gvkries commented Jul 2, 2024

Also any additional data of a derived type gets lost as well, if you do not add the property for additional JSON data in the Query (with the JsonExtensionAttribute).

Note that all of this happens, because all queries are stored in a single document and everything gets re-serialized even if you don't edit a specific query.

After editing the document with your changes, you will only have something like this:

Before:

{
    "Queries": {
        "Test": {
            "$type": "OrchardCore.Queries.Sql.SqlQuery, OrchardCore.Queries",
            "Template": "SELECT * FROM foobar",
            "ReturnDocuments": false,
            "Name": "Test",
            "Source": "Sql",
            "Schema": "{\r\n    \u0022type\u0022: \u0022ContentItem/BlogPost\u0022\r\n}"
        },
        "RecentBlogPosts": {
            "$type": "OrchardCore.Search.Lucene.LuceneQuery, OrchardCore.Search.Lucene",
            "Index": "Search",
            "Template": "{\r\n  \u0022query\u0022: {\r\n    \u0022term\u0022: { \u0022Content.ContentItem.ContentType\u0022: \u0022BlogPost\u0022 }\r\n  },\r\n  \u0022sort\u0022: {\r\n    \u0022Content.ContentItem.CreatedUtc\u0022: {\r\n      \u0022order\u0022: \u0022desc\u0022,\r\n      \u0022type\u0022: \u0022double\u0022\r\n    }\r\n  },\r\n  \u0022size\u0022: 3\r\n}\r\n",
            "ReturnContentItems": true,
            "Name": "RecentBlogPosts",
            "Source": "Lucene",
            "Schema": "{\r\n    \u0022type\u0022: \u0022ContentItem/BlogPost\u0022\r\n}"
        }
    },
    "Identifier": "40n8jfvfaty50ybnrnexhr15nz"
}

After:

{
    "Queries": {
        "Test": {
            "$type": "OrchardCore.Queries.Sql.SqlQuery, OrchardCore.Queries",
            "Template": "SELECT * FROM foobarx",
            "ReturnDocuments": false,
            "Name": "Test",
            "Source": "Sql",
            "Schema": "{\r\n    \u0022type\u0022: \u0022ContentItem/BlogPost\u0022\r\n}"
        },
        "RecentBlogPosts": {
            "Name": "RecentBlogPosts",
            "Source": "Lucene",
            "Schema": "{\r\n    \u0022type\u0022: \u0022ContentItem/BlogPost/XXX\u0022\r\n}"
        }
    },
    "Identifier": "4ej5anpdsbwgn55c2x0053kvwg"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when disabling Lucine or elastic search feature
2 participants