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.

Sign up for GitHub

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

Jump to bottom

Update DAVx5 in AuthToken.vue #25084

Merged
merged 1 commit into from
Jan 14, 2021
Merged

Update DAVx5 in AuthToken.vue #25084

merged 1 commit into from
Jan 14, 2021

Conversation

rfc2822
Copy link
Contributor

@rfc2822 rfc2822 commented Jan 11, 2021

Hi,

Please have a look at these changes:

  1. It's named DAVx5 (only very old clients have DAVdroid in the User-Agent)
  2. Use passive matching (?:droid|x5) because otherwise, "x5" is recognized and shown as operating system
  3. Also include the version postfix which contains information like "-beta2", which is important for the user

Is renaming the constant davDroid to davx5 somehow problematic?

@rfc2822
Copy link
Contributor Author

rfc2822 commented Jan 11, 2021 •
edited
Loading

This is how DAVx5 is shown before the MR:

Bildschirmfoto von 2021-01-11 19-33-01

  1. "DAVx5" is shown as "DAVdroid"
  2. "-beta2" is missing next to "3.3.8"
  3. "x5" is shown as operating system

Do you need any other information to review the changes?

@kesselb kesselb added 3. to review Waiting for reviews enhancement labels Jan 12, 2021
@ChristophWurst ChristophWurst added this to the Nextcloud 21 milestone Jan 12, 2021
ChristophWurst
ChristophWurst approved these changes Jan 12, 2021
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Sounds good!

I'm also fine if we leave the old user agent in for any older clients? Or is that too unlikely that someone still uses the davDroid app?

@rfc2822
Copy link
Contributor Author

rfc2822 commented Jan 12, 2021

Yes I'd keep the (?:droid|x5) (as I have done in the MR) which should match both ones, just to be sure. I think they should both be displayed as DAVx5, which is the official name since 1.1.2019.

@rullzer rullzer mentioned this pull request Jan 14, 2021
Merged
14 tasks
@ChristophWurst
Copy link
Member

/compile amend /

@rfc2822 @npmbuildbot-nextcloud
Update DAVx5 in AuthToken.vue
bcd1546 
1. It's named DAVx5 (only very old clients have DAVdroid in the User-Agent)
2. Use passive matching (?:droid|x5) because otherwise, "x5" is recognized and shown as operating system
3. Also include the version postfix which contains information like "-beta2", which is important for the user

Signed-off-by: npmbuildbot-nextcloud[bot] <npmbuildbot-nextcloud[bot]@users.noreply.github.com>
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 14, 2021
@rullzer rullzer mentioned this pull request Jan 14, 2021
Merged
15 tasks
@rullzer rullzer merged commit 5a141bd into nextcloud:master Jan 14, 2021
@welcome
Copy link

welcome bot commented Jan 14, 2021

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@GretaD GretaD GretaD approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement
Projects
None yet
Milestone
Nextcloud 21
Development

Successfully merging this pull request may close these issues.
6 participants
@rfc2822 @ChristophWurst @GretaD @skjnldsv @rullzer @kesselb