Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

Add automatic coloring of tags #84

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

jayme-github
Copy link
Contributor

Please see this as proposal on how to implement automatic/random colors
for tags while keeping the current set of hard coded colors in place (at
least in the frontend).

Bear with me as I have even less Angular knowledge than Django and just
tried to get away with as few changes as possible. :-) AIUI you want to
change to a color picking system anyways in the future, which could also
play well with this.

fixes #51

Please see this as proposal on how to implement automatic/random colors
for tags while keeping the current set of hard coded colors in place (at
least in the frontend).

Bear with me as I have even less Angular knowledge than Django and just
tried to get away with as few changes as possible. :-) AIUI you want to
change to a color picking system anyways in the future, which could also
play well with this.

fixes the-paperless-project#51
@jonaswinkler
Copy link
Owner

Hello, and thanks again for contributing! (Don't worry too much about the coverage report)

The next release is already kinda packed with goodies, and I want to get this wrapped up, so I'll look into this alter.

@jayme-github
Copy link
Contributor Author

Hello, and thanks again for contributing! (Don't worry too much about the coverage report)

The next release is already kinda packed with goodies, and I want to get this wrapped up, so I'll look into this alter.

Sure, no rush. Let me know if you want anything changed.


getColor(id) {
return TAG_COLOURS.find(c => c.id == id)
var color = TAG_COLOURS.find(c => c.id == id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we mix US and British English spelling for color (colour)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I'm happy to clean that up once/if the general approach is accepted.

@jayme-github
Copy link
Contributor Author

@jonaswinkler did you had the time to think about this by chance? Would you accept the approach in general? I could rebase and fix the spelling then.

@jonaswinkler
Copy link
Owner

@bauerj

I want to look into this again and need some input. The gist of it is that the field Tag.colour will not return an indexed color anymore, but rather a hex string #ffabab. (Fancy how github parses that!)

As far as I'm aware, your app is the only client that extensively uses the API. Are you using this field in your app? Would you mind me changing that? The more compatible approach would be to

  • Add colour_code field
  • Keep colour as a read only field that tries to return one of the predefined colors close to what's saved in colour_code.

@bauerj
Copy link
Contributor

bauerj commented Feb 24, 2021

Yes, the app is currently using that field. Your workaround seems useful although it probably needs a lot of code to find the closest original colour.

I guess colour could just return a fixed number for non-default colours. This wouldn't break any important functionality and only for a very short period of time. I will implement an agreed-upon change quickly so we can have the app ready before users can use the new feature on the server-side.

App updates are usually installed within weeks at most so I guess you can drop colour entirely in a few versions.

For future changes it might be better to send an X-API-Version header or something. This could enable the app to prompt users to update if their server is too new.

@jonaswinkler
Copy link
Owner

although it probably needs a lot of code to find the closest original color. I guess colour could just return a fixed number for non-default colors.

True.

For future changes it might be better to send an X-API-Version header or something. This could enable the app to prompt users to update if their server is too new.

Hm. I'll add API versioning. Time to learn something new! There's support for that in the framework, and I'm apparently able to alter the behavior based on what version is requested. Old clients get the old version, new clients get fancy color codes.

bauerj added a commit to bauerj/paperless_app that referenced this pull request Feb 24, 2021
@bauerj
Copy link
Contributor

bauerj commented Feb 24, 2021

Alright, I implemented a client-side change assuming the JSON key will be named "colour_code". It still works for the old API so I'm going to push an update.

I wonder if there is an easier way than to maintain backwards-compatibility client-side as well as server-side 😄

@jonaswinkler
Copy link
Owner

Alright, I implemented a client-side change assuming the JSON key will be named "colour_code". It still works for the old API so I'm going to push an update.

I wonder if there is an easier way than to maintain backwards-compatibility client-side as well as server-side

Wait.

@bauerj
Copy link
Contributor

bauerj commented Feb 24, 2021

Okay 😄

I ran into a new kind of dependency hell anyway. Is there anything else that I would need to change?

@jonaswinkler
Copy link
Owner

jonaswinkler commented Feb 24, 2021

I am about to implement versioning according to the documentation (https://www.django-rest-framework.org/api-guide/versioning/#acceptheaderversioning). This will ensure backward compatibility of all (older) clients with all servers.

  • The default would be to serve v1 (which is the current state, and will serve compatible colour fields).
  • If the version is specified, the behavior of the server will change according to the requested version.
  • v2 and onward would essentially be v1, but with the changes to the Tag model. (color_code present, colour removed)
  • Maybe I'll just name that field color. colour_code was just an idea to maintain backward compatibility anyway.

@bauerj Adding that versioned Accept header to every request is fairly simple in the front end - how's that on your end?

@bauerj
Copy link
Contributor

bauerj commented Feb 25, 2021

Looks like the app has to maintain backwards-compatibility for old servers and the server has to maintain backwards-compatibility for old clients then 😅

I wonder what happens if the client requests an API version 3. Would the server just use the v2 API or would it raise an error?

Adding that versioned Accept header to every request is fairly simple in the front end - how's that on your end?

Yes, that would be simple as well.

@jonaswinkler
Copy link
Owner

jonaswinkler commented Feb 25, 2021

I wonder what happens if the client requests an API version 3. Would the server just use the v2 API or would it raise an error?

Newer servers will respond with "406 Not Acceptable", and an error message in the body.

Looks like the app has to maintain backwards-compatibility for old servers and the server has to maintain backwards-compatibility for old clients then

Not at all! You simply build your app to target one of the versions, and expect the server to respond with data from that version. The apps that are currently running out there will continue to request v1. When you update your app to use the new color fields, simply request version 2, and the server will serve data from that version, even if newer API versions appear in the future.

I'll update the API docs with specifics once that's done.

@jonaswinkler
Copy link
Owner

image

This took way too much time.

@shamoon
Copy link
Contributor

shamoon commented Feb 25, 2021

This took way too much time.

Heh, on the plus side it looks dope.

@jonaswinkler jonaswinkler merged commit 7233695 into jonaswinkler:dev Feb 25, 2021
@jonaswinkler
Copy link
Owner

For future changes it might be better to send an X-API-Version header or something. This could enable the app to prompt users to update if their server is too new.

With this you could verify whether your app is compatible with any given server, right? X-API-Version not present -> not compatible, X-API-Version present -> compatible if this version is high enough. Correct?

@bauerj
Copy link
Contributor

bauerj commented Feb 26, 2021

Looks like the app has to maintain backwards-compatibility for old servers and the server has to maintain backwards-compatibility for old clients then

Not at all! You simply build your app to target one of the versions, and expect the server to respond with data from that version. The apps that are currently running out there will continue to request v1. When you update your app to use the new color fields, simply request version 2, and the server will serve data from that version, even if newer API versions appear in the future.

Well, for the time being we still need to support Paperless and Paperless NG API version 1 servers. I don't think it would be a good idea to force users to upgrade their server after an automatic app update.

While app updates are usually installed by almost all users after 1-2 days, server updates can take a lot longer since there is no integrated update mechanism.

However, this versioning scheme is still useful for other clients.

I wonder what happens if the client requests an API version 3. Would the server just use the v2 API or would it raise an error?

Newer servers will respond with "406 Not Acceptable", and an error message in the body.

I'm not sure if this is a good idea. Imagine we have API version 10 some time in the future but the user would still use an old server that only supports version 2 of the API. Then the app would have to request data using API versions 10-2, causing 8 requests to find the common API version. Or does the 406 response contain an X-API-Version header with the newest API?

@jonaswinkler
Copy link
Owner

I'm not sure either how to do this properly.

Every response to authenticated requests will contain X-Api-Version.

@jonaswinkler
Copy link
Owner

What should I do when the client requests a version that the server does not (yet) have?

The current state of things on the dev branch is described in the documentation. https://paperless-ng.readthedocs.io/en/dev/api.html#api-versioning

mweimerskirch pushed a commit to mweimerskirch/paperless-ng that referenced this pull request Feb 18, 2022
…uage-da-dk

Enabled the Danish (da-dk) translations
tribut pushed a commit to tribut/paperless-ng that referenced this pull request Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants