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

Improved docs of Plone REST API JavaScript Client #5576

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
119 changes: 29 additions & 90 deletions docs/source/client/actions/aliases.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,120 +7,59 @@ Handles transitive references (for example a -> b, b -> c becomes a -> c) and ig

## Get aliases list

### Query function
```{js:function} getAliasesListQuery

Use the `getAliasesListQuery` function to get the query for fetching the aliases list.

### Hook

Use the `useGetAliasesList` hook to get the aliases list.
:hook: `useGetAliasesList`
```

## Get aliases

### Query function
```{js:function} getAliasesQuery(path)

Use the `getAliasesQuery` function to get the query for fetching the aliases for a page.

### Hook

Use the `useGetAliases` hook to get the aliases for a page.

### Parameters

- **path**: string

- **Required:** Yes
:arg string path: Description of the `path` parameter.
:hook: `useGetAliases`
```

## Add aliases for many pages

### Mutation function
```{js:function} createAliasesMutation(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The next three went sideways. See example: https://deploy-preview-5576--volto.netlify.app/client/actions/aliases.html#add-aliases-for-many-pages

I'm not sure how to document the structure of an object as an argument. If you don't have any better idea, I'd try this (untested):

:arg array data: It can have the following fields:
  - items: An array of objects with the following fields:
    - path: string
    - redirect_to: string
    - [datetime]: string

You might need to indent the list items to the right amount. I don't know if Markdown for list items will even work in the value for arguments.

The ultimate fallback would be to convert the object into narrative text, which is not great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, lets try it and see if it's better

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also do make docs-livehtml to preview docs in real-time. It's a lot faster than waiting for Netlify. It still doesn't look quite right. https://deploy-preview-5576--volto.netlify.app/client/actions/aliases.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stevepiercy I tried doing make docs-livehtml but unfortunately I am getting this error :
/usr/bin/sh: line 2: bin/python: No such file or directory make: *** [Makefile:99: bin/python] Error 127

Copy link
Collaborator

Choose a reason for hiding this comment

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

Which way did you use to set up your environment for Volto docs? There are two ways, depending on the scope.

It looks like you do not have a Python virtual environment installed, according to this:

/usr/bin/sh: line 2: bin/python: No such file or directory make: *** [Makefile:99: bin/python] Error 127

Line 99 in Makefile has this:

	python3 -m venv . || virtualenv --clear --python=python3 .

which means "Create a Python 3 virtual environment in the current directory, if possible, else use virtualenv to wipe and recreate a Python 3 virtual environment." It fails because you either do not have Python installed on your system or, if you do have it installed, Python is not in your system $PATH.

If you do not have Python installed, see the advice to install Python. Else you will need to ensure that your $PATH is configured to point to a Python on your system. Please let me know.


Use the `createAliasesMutation` function to get the mutation for adding aliases for many pages.

### Hook

Use the `useCreateAliases` hook to add aliases for many pages.

### Parameters

- **data**: object

- **Required:** Yes
- It can have the following fields:

`items: object[]`:

- **Required:** Yes
- An array of objects with the following fields:

`path: string`

- **Required:** Yes

`datetime: string`

- **Required:** No

`redirect_to: string`

- **Required:** Yes
:arg object data : It can have the following fields:
:arg object[] items: An array of objects with the following fields:
:arg string path:
:arg string redirect_to:
:arg string [datetime]:
:hook: `useCreateAliases`
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we have an easy and useful way to show data structures in narrative text through docstrings. Here's my latest attempt, but it is still unsatisfactory:

## Add aliases for many pages

```{js:function} createAliasesMutation(data)

Use the `createAliasesMutation` function to get the mutation for adding aliases for many pages.

:arg array data: It can have the following fields:

                 `items`
                 : An array of objects with the following fields:

                     -   `path`: string
                     -   `redirect_to`: string
                     -   \[`datetime`\]: string

:hook: `useCreateAliases`
Screenshot 2024-01-07 at 5 21 35 AM

That Markdown is horrible and difficult to write and maintain, and the result still does not look correct.

Perhaps we should instead adopt the practice of what we do in the Plone REST API, where we show the data structure to send as a POST request with an expected response?

https://6.docs.plone.org/plone.restapi/docs/source/endpoints/aliases.html#adding-url-aliases-in-bulk

@Hrittik20 what do you think?

Before going further, I think I want to chat with @sneridagh to discuss how we can both improve this package's documentation and better integrate with the Plone REST API docs. That conversation might yield a better direction. I ask for your patience until we can work through this. It might take until our next Volto Team meeting, which you are welcome to join. See https://discord.com/channels/786421998426521600/787308038050545666/1191701430642430054 for details.

I apologize that I did not realize that this would become more complicated, but I want to thank you for helping me come to this realization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stevepiercy Absolutely, I agree with your suggestion to improve the documentation following the Plone REST API approach. Thanks for your assistance throughout this matter. Also, can you share the discord link again (link expired probably)?

Copy link
Collaborator

Choose a reason for hiding this comment

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


## Add aliases for a page

### Mutation function
```{js:function} createAliasesMutation(path, data)

Use the `createAliasesMutation` function to get the mutation for adding aliases for a page.

### Hook

Use the `useCreateAliases` hook to add aliases for a page.

### Parameters

- **path**: string

- **Required:** Yes

- **data**: object

- **Required:** Yes
- It can have the following fields:

`items: object[]`:

- **Required:** Yes
- An array of objects with the following fields:

`path: string`

- **Required:** Yes
:arg string path: Description of the `path` parameter.
:arg object data: It can have the following fields:
:arg object[] items: An array of objects with the following fields:
:arg string path:
:hook: `useCreateAliases`
```

## Delete aliases

### Mutation function
```{js:function} deleteAliasesMutation(path, data)

Use the `deleteAliasesMutation` function to get the mutation for deleting aliases for a page.

### Hook

Use the `useDeleteAliases` hook to delete aliases for a page.

### Parameters

- **path**: string

- **Required:** Yes

- **data**: object

- **Required:** Yes
- It can have the following fields:

`items: object[]`:

- **Required:** Yes
- An array of objects with the following fields:

`path: string`

- **Required:** Yes
:arg string path: Description of the `path` parameter.
:arg object data: It can have the following fields:
:arg object[] items: An array of objects with the following fields:
:arg string path:
:hook: `useDeleteAliases`
```
1 change: 1 addition & 0 deletions docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@
myst_enable_extensions = [
"deflist", # You will be able to utilise definition lists
# https://myst-parser.readthedocs.io/en/latest/syntax/optional.html#definition-lists
"fieldlist", # For docstrings,
"linkify", # Identify “bare” web URLs and add hyperlinks.
"colon_fence", # You can also use ::: delimiters to denote code fences,\
# instead of ```.
Expand Down