[#119] Change set-field Web-API

sancho20021 commented 2 years ago

Description

Problem: set-field route was used both for changing visibility and setting new fields. This resulted in additional handling of the situation where both field contents and visibility option are missing.

Solution: require contents for setting field and allow changing visibility for convenience. Add set-field-visibility request which modifies only visibility.

Question?

Currently in set-field-visibility I pass the value in the body of the request. Maybe it would be better to pass it in the request string itself:

set-field-visibility/entry-path/field/private

I don't know.

Related issue(s)

Fixed part of #119. Refactor of CLI API still needed.

:white_check_mark: Checklist for your Pull Request

Related changes (conditional)

Tests
- [x] If I added new functionality, I added tests covering it.
- [x] If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
Documentation
- [x] I checked whether I should update the docs and did so if necessary:
- README
- Haddock

Stylistic guide (mandatory)

[x] My commits comply with the policy used in Serokell.
[x] My code complies with the style guide.

Kariel-Myrr commented 2 years ago

Seems in issue description field visibility should be a query parametr

These issues can be solved by having a single endpoint that accepts an optional ?visibility=public/private query parameter and an optional FieldContents.

sancho20021 commented 2 years ago

Seems in issue description field visibility should be a query parametr

Yes but that was an initial proposal, we later decided to split our queries into set-field with indeed an optional visibility parameter and set-field-visibility.

I don't want the set-field-visibility to take visibility as a parameter because it would require repeating the word visibility two times: /set-field-visibility/?visibility=private

Kariel-Myrr commented 2 years ago

Ok, IMO it would be more logical to make it query. JSON looks like overshot. And seems nothing chain us to JSON format

sancho20021 commented 2 years ago

Ok, IMO it would be more logical to make it query. JSON looks like overshot. And seems nothing chain us to JSON format

Yeah, will redo.

sancho20021 commented 2 years ago

@Kariel-Myrr I moved visibility from JSON body to the query string. Now a request looks like

/set-field-visibility/public

sancho20021 commented 2 years ago

This PR is closed due to existing https://github.com/serokell/coffer/pull/130.

serokell / coffer