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

IBX-8369: Fix REST LocationList #2445

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

Conversation

adriendupuis
Copy link
Contributor

@adriendupuis adriendupuis commented Jul 22, 2024

Question Answer
JIRA Ticket IBX-8369
Versions master, 4.6
Edition All

The old definition is the one from the PHP class. The REST output has nothing in common with the PHP class:

It's also important to notice that REST LocationList is not a list of https://github.com/ibexa/rest/blob/main/src/lib/Server/Output/ValueObjectVisitor/Location.php

Note: Sample case, if the right syntax is found, similar ones could be fixed (BookmarkList , ContentTypeList, UrlAliasRefList ObjectStateGroupList, …)

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Description metadata is up to date
  • Redirects cover removed/moved pages
  • Code samples are working
  • PHP code samples have been fixed with PHP CS fixer
  • Added link to this PR in relevant JIRA ticket or code PR

Comment on lines 538 to 540
type: Ref[]
xml:
name: LocationList
Copy link
Contributor Author

@adriendupuis adriendupuis Jul 22, 2024

Choose a reason for hiding this comment

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

Inspired by https://dzone.com/articles/the-x-factor-raml-with-xml-data-format but I don't feel it's the right way. It misses that the "Ref" nodes are named Location.

<?xml version="1.0" encoding="UTF-8"?>
<LocationList media-type="application/vnd.ibexa.api.LocationList+xml" href="/api/ibexa/v2/content/objects/1/locations">
 <Location media-type="application/vnd.ibexa.api.Location+xml" href="/api/ibexa/v2/content/locations/1/2/42"/>
</LocationList>

@adriendupuis adriendupuis added Wait with merge PRs that shouldn't be merged instantly Work in progress labels Jul 23, 2024
@adriendupuis adriendupuis changed the base branch from master to fix-rest-types July 23, 2024 08:45
Comment on lines 1085 to 1089
LocationListXML:
type: LocationRefXML[]
xml:
name: LocationList
properties:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's like LocationList node shouldn't have properties…

Message: Property 'properties' not supported in a RAML 1.0 array node

<?xml version="1.0" encoding="UTF-8"?>
<LocationList media-type="application/vnd.ibexa.api.LocationList+xml" href="/api/ibexa/v2/content/objects/1/locations">
 <Location media-type="application/vnd.ibexa.api.Location+xml" href="/api/ibexa/v2/content/locations/1/2/42"/>
</LocationList>

adriendupuis and others added 14 commits July 30, 2024 10:20
…2418)

* Ibexa Cloud product guide page added

* Content added

* Image added

* Content & images added

* Image fixed

* Content added

* Vale suggestions

* Fixes after review

* Content added

* Fixes

* Updates & content added

* Updates

* Sentence updated

* Content added; fixes

* Kafka removed

---------

Co-authored-by: Julita Falcon Dusza <julitafalcondusza@Users-MacBook-Pro.local>
* Mentioned Symfony performance article in the doc

* Applied suggestion

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>

---------

Co-authored-by: Adrien Dupuis <61695653+adriendupuis@users.noreply.github.com>
* Added documentation about env function for migrations

* Changed from tag names to function names
adriendupuis and others added 3 commits July 30, 2024 10:21
---------

Co-authored-by: julitafalcondusza <117284672+julitafalcondusza@users.noreply.github.com>
@adriendupuis adriendupuis changed the title Fix REST LocationList IBX-8369: Fix REST LocationList Aug 2, 2024
Base automatically changed from fix-rest-types to master August 26, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Wait with merge PRs that shouldn't be merged instantly Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants