-
-
Notifications
You must be signed in to change notification settings - Fork 318
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
Added raw values of attributes to JSON API #988
Conversation
… ends than the web front end
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
// Maybe, it would be reasonable to migrate the web frontend to use the raw formats, too Actually, I plan to nuke completely the full JS front-end. :) |
@@ -24,7 +24,9 @@ public function toReturnArray(): array | |||
'public' => $this->get_public(), | |||
'album' => $this->album_id !== null ? strval($this->album_id) : null, | |||
'width' => strval($this->width), | |||
'width_raw' => $this->width !== null ? $this->width : -1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for a late feedback, but I don't understand the value of width_raw
and height_raw
? They will contain exactly the same data as width
and height
, only as integers rather than strings, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. The main reason for these changes and other PRs (see PR #991 and PR #990) is to please clients (such as Androids Retrofit library) which insist on strong and correct typing. At the same time I didn't want to spend too much time on changing the current web frontend. As @ildyria mentioned he wants to nuke and rewrite it anyway. So I thought the most efficient way would be
- Add new attributes (with the correct type) to the JSON response even if this means that information is transported redundantly
- This way I don't need to touch the current web frontend
- I can concentrate on proceeding with the Android client
- Let @ildyria do the rewriting of the (new?) web frontend and hope that he will also use the "new" attributes
- Remove the old, redundant attributes in the future if they are not used any more by the web frontend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let @ildyria do the rewriting of the (new?) web frontend and hope that he will also use the "new" attributes
I'm sorry to disappoint you but I will not use the REST api at all for the new front-end.
I wouldn't say that the JS one is depreciated because what I have is no-where near good enough to replace it yet. But in the end the REST api would only be useful for people using e.g. your Android client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry to disappoint you but I will not use the REST api at all for the new front-end.
No disappointment, but the opposite. Even better I would say. If the new frontend won't use the REST API, all "legacy" attributes will be able to be removed after that. Then, only the "raw" attributes which are a thin wrapper around the database can remain.
I wouldn't say that the JS one is depreciated because what I have is no-where near good enough to replace it yet.
@ildyria Do you have an rough estimate when you will have finished that? Are we talking about one or two months or more than a year? IMHO, an API should normally be kept clean with no redundancy. I only took that approach (add new attributes, keep the old attributes in place and don't touch them), because I thought this would only last a short term, intermediate period until the new web frontend is finished. However, if you say that it will probably last more then a short time, then I would also take the effort and have a look at the current front end and patch that to use the new attributes such that those old attributes can be removed from the API sooner than later.
But in the end the REST api would only be useful for people using e.g. your Android client.
No problem, see above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do expect it to take me 3 months if I were to work on it all my weekends. :'D
Because I basically need to rewrite everything in such way that visually it is similar to the JS front-end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, from my perspective that's amazingly fast. Anyway, I will pay more attention to also align the current web front to the changes of the REST API such that the REST API remains clean and does not become cluttered with duplicate attributes.
The raw values are required by other clients than the web-front end, e.g. an android client which is currently under development