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

Support for Tag backend. #396

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from
Draft

Conversation

axl8713
Copy link
Collaborator

@axl8713 axl8713 commented Jan 24, 2025

Changes as per #382:

  • Added CRUD operations for Tag entity under the Resource context:

    • create: POST /geostore/rest/resources/tag
    • get all: GET /geostore/rest/resources/tag[?page&entries][?nameLike]
    • get: GET /geostore/rest/resources/tag/{id}
    • update: PUT /geostore/rest/resources/tag/{id}
    • delete: DELETE /geostore/rest/resources/tag/{id}
  • Added association operations for Tags under the Resource context:

    • associate resource: POST /geostore/rest/resources/tag/{id}/resource/{resourceId}
    • unassociate resource: DELETE /geostore/rest/resources/tag/{id}/resource/{resourceId}
  • Added support to showing tags for the resources in ExtJS get all result:

    • request: GET /geostore/rest/extjs/search/list?includeTags=true
    • response:
    {
      "ExtResourceList": {
        "ResourceCount": 3,
        "Resource": [
          {
            "advertised": false,
            "category": {
              "id": 1,
              "name": "cat1"
            },
            "creation": "2025-01-17T19:04:03.379+00:00",
            "creator": "Y",
            "editor": "editore",
            "id": 101,
            "name": "user owned readonly",
            "tags": {
              "color": "red",
              "description": "R",
              "id": 100,
              "name": "Red"
            },
            "canEdit": true,
            "canDelete": true,
            "canCopy": true
          },
         [...]
  • Added support to showing tags for the single resource in ExtJS:

    • request: GET /geostore/rest/extjs/resource/100?includeTags=true
    • response:
    <ShortResource>
        <advertised>true</advertised>
        <canDelete>true</canDelete>
        <canEdit>true</canEdit>
        <creation>2025-01-17T19:04:03.379+00:00</creation>
        <creator>X</creator>
        <editor>editore</editor>
        <id>100</id>
        <name>uno</name>
        <attributeList/>
        <securityRuleList/>
        <tagList>
            <Count>2</Count>
            <Tag>
                <color>#fdfdfd</color>
                <description>grey</description>
                <id>14</id>
                <name>grey_</name>
            </Tag>
            <Tag>
                <color>red</color>
                <description>R</description>
                <id>100</id>
                <name>Red</name>
            </Tag>
        </tagList>
    </ShortResource>

Other changes:

  • Added tag resource filtering support:
      <TAG>
          <operator>IN</operator>
          <names>Red</names>
      </TAG>
  • Published get all tags operation to anonymous callers

Closes #382

@axl8713 axl8713 requested a review from afabiani January 24, 2025 10:46
@axl8713 axl8713 self-assigned this Jan 24, 2025
@axl8713 axl8713 requested a review from allyoucanmap January 24, 2025 10:46
@axl8713
Copy link
Collaborator Author

axl8713 commented Jan 28, 2025

Will update API wiki after the approval.

@axl8713 axl8713 marked this pull request as ready for review January 28, 2025 13:29
@afabiani
Copy link
Member

@tdipisa on my side the PR looks good, but this one introduces a change on the model. Before merging to the main branch you should consider the possible breaking changes on the current installations.

@tdipisa
Copy link
Member

tdipisa commented Jan 28, 2025

@tdipisa on my side the PR looks good, but this one introduces a change on the model. Before merging to the main branch you should consider the possible breaking changes on the current installations.

Understood @afabiani. That would be a test per se, indeed. If the review is ok, let's merge, we can always revert it if necessary. Maybe, just let me merge it in the right moment.

@tdipisa tdipisa marked this pull request as draft January 28, 2025 17:36
@tdipisa
Copy link
Member

tdipisa commented Jan 28, 2025

converted to draft for the moment.

@tdipisa tdipisa requested review from tdipisa and removed request for allyoucanmap January 29, 2025 08:56
@tdipisa tdipisa added this to the 2025.01.00 milestone Jan 29, 2025
Copy link

@allyoucanmap allyoucanmap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axl8713 please include in this PR all the additional fixes:

Then you can close the PRs:

@tdipisa
Copy link
Member

tdipisa commented Jan 31, 2025

@axl8713 please include in this PR all the additional fixes:

Then you can close the PRs:

@axl8713 I agree, if possible that would be better to have all in one also for the history as it is a new feature.

This was referenced Jan 31, 2025
@allyoucanmap
Copy link

@tdipisa I'm reporting the problem related to the filtering using the IN operator with tags containing special chars. The filter IN operator expect a list of names (Tag1,Tag2) but if a tag contains , or " (eg. "Tag", Ta,g) the filter will not work as expected. @axl8713 introduced a fix for , 024f94a by wrapping the name with " but this does not solve yet the case of a tag name using " as wrapper

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

Successfully merging this pull request may close these issues.

Support for Tag backend
4 participants