add column types for IPv4 and IPv6 addresses, compatible with both text and numeric filters, thanks to natural sort

kirisakow commented 4 months ago

This is an attempt to respond to issue #402 (and #981) by implementing TextIPv4Address and TextIPv6Address column types that are compatible with both text and numeric FilterIds (all nine of them), thanks to natural sort.

This is my first PR on this project, so please feel free to kindly point out anything I may have missed or done wrong.

enjeck commented 4 months ago

@kirisakow Thanks for working on this. Coincidentally, I've also been working on a new column type (for users/groups) at: https://github.com/nextcloud/tables/pull/1090. Maybe the files and feedback I'm getting over there helps :smile:

enjeck commented 4 months ago

@kirisakow While working on https://github.com/nextcloud/tables/pull/1090, I wrote down some notes of files I potentially had to change/add. I think not everything is applicable here, but might be helpful:

Frontend

In src/shared/components/ncTable/mixins/columnsTypes, we create a new column type that specifies:
- properties the column has
- converting value to string representation (and using for search and filter operations)
- sort function
In src/shared/components/ncTable/mixins/magicFields.js, we specify if the new column fits any particular magicfield
In src/shared/components/ncTable/mixins/filters.js, we specify if the new column should be shown for any particular filter
In src/shared/components/ncTable/mixins/columnParser.js, we specify that this new column is valid
In src/shared/components/ncTable/mixins/columnHandler.js, add the column and subtypes if any
In src/modules/modals/EditColumn.vue and src/modules/modals/CreateColumn.vue, specify how to handle column creation and editing
in src/modules/main/partials/ColumnFormComponent.vue, add the column
In src/modules/main/partials/ColumnTypeSelection.vue, add icon and text that will appear when user initially chooses a column type
In src/shared/constants.js , specify new column type
In src/shared/components/ncTable/mixins/columnClass.js, maybe add Abstract column
In src/shared/components/ncTable/mixins/columnHandler.js, add datatype
In src/shared/components/ncTable/mixins/columnParser.js, add type
In src/shared/components/ncTable/mixins/metaColumns.js, maybe add meta type
In src/shared/components/ncTable, add new TableCell component file for rendering the type in the table.
In src/shared/components/ncTable/partials/columnTypePartials/forms, add a new Form component that will be inserted into the create/edit form when we try to create a new column for this new type. Will have to create this for subtypes too
In src/shared/components/ncTable/partials/rowTypePartials, add a new Form component that will be inserted into the create/edit form when we try to create a new row for this new type. Will have to create this for subtypes too, if there are any

Backend

In lib/Service/ColumnService.php, specify properties
In lib/Service/ColumnTypes, add a new column type
In `lib/Db/Column.php, add new column type
Add column properties to lib/Service/TableTemplateService.php
Add to lib/Controller/ColumnController.php the properties for new type
Add type to lib/Controller/ApiColumnsController.php
Add column details to lib/ResponseDefinitions.php
Add column type to lib/Capabilities.php
Add route to create column to appinfo/routes.php
Add properties to lib/Controller/Api1Controller.php
Add new column type file to lib/Db/ColumnTypes
Add to new column data lib/Db/LegacyRowMapper.php
Add to lib/Db/Row2Mapper.php
Add new RowCell and RowCellMapper php file for the new column type to lib/Db
Add column type to lib/Helper/ColumnsHelper.php
Update migration files to include new type. Look at all files at lib/Migration most especially lib/Migration/Version000000Date20210921000000.php and lib/Migration/Version000700Date20230916000000.php
Maybe add to lib/Service/ImportService.php

Others

Ensure types are added to src/types/openapi/openapi.ts
Add bdd test to tests/integration/features/APIv2.feature
Ensure types are added to openapi.json
Write e2e tests

kirisakow commented 4 months ago

@enjeck Thank you for the input, that was of much help!

I assume development is now finished for the

[x] front-end part;
[x] back-end part;

Still, a few TODOs remain — like things I wasn't sure about:

[ ] I'd appreciate for the core logic code to be reviewed by someone possessing more experience than me:
- [ ] src/shared/components/ncTable/mixins/columnsTypes/textIPv4Address.js
- [ ] src/shared/components/ncTable/mixins/columnsTypes/textIPv6Address.js
[ ] specify valid IPv4 and IPv6 regex patterns, as I wasn't sure where and how to declare a value of textAllowedPattern;
[ ] every thing tests-related — unit tests, integration tests;
[ ] every thing OpenAPI-related*;
[ ] every thing migrations-related*;

* probably nothing to do about OpenAPI or migrations, as this feature merely adds two subtypes to the existing text type.

github-actions[bot] commented 3 months ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

nextcloud / tables