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.

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

Make use of new productversion field if available #7045

Merged
merged 2 commits into from
May 24, 2022

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented May 23, 2022

Description

oCIS has introduced a new productversion field to announce it's correct version while maintaining a fake 10.x.x version in the versionstring field to keep clients compatible. We're using the new productversion field when it exists and use versionstring as a fallback. Thus the backend product information prints the correct oCIS version now.

Since it seems to be encouraged to use capabilities.core.status this PR switches over to using that instead of the version object. 🤷

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@@ -6,12 +6,12 @@ export const getWebVersion = (): string => {
}

export const getBackendVersion = ({ store }: { store: Store<unknown> }): string => {
const backendVersion = store.getters.user.version
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, where did user.version come from in the past?

Copy link
Member Author

Choose a reason for hiding this comment

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

the capabilities response is extracted into the user store module as capabilities and version keys. see

SET_CAPABILITIES(state, data) {
(yes, it's weird and ugly 😆 don't know why it is like that).

@ownclouders
Copy link
Contributor

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/25804/12/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/25804/tracing/unstructured-collection-of-testable-space-interactions-alice-2022-5-23-03-43-40.zip

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

LGTM, unclear why the E2E tests for oCIS fail here though. Also, should we combine this PR with an oCIS commit ID version bump to reap the benefits of owncloud/ocis#3805 ?

@micbar
Copy link
Contributor

micbar commented May 24, 2022

@kulmann Now you have all the options, see owncloud/ocis#3854

const edition = backendVersion.edition
const product = backendStatus.product || 'ownCloud'
const version = backendStatus.productversion || backendStatus.versionstring
const edition = backendStatus.edition
return `${product} ${version} ${edition}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Just had a random shower thought on whether we should insert (development) here if process.env.NODE_ENV === development

Copy link
Contributor

Choose a reason for hiding this comment

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

To detect whether the bundle-watched dist is being used or the shipped web in oCIS - I've been adding a lot of console.log's recently to debug caching

Copy link
Member Author

Choose a reason for hiding this comment

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

oha, nice! good idea :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

will do that in a followup to unblock the next ocis beta

@kulmann
Copy link
Member Author

kulmann commented May 24, 2022

@kulmann Now you have all the options, see owncloud/ocis#3854

that raises the question: which one is preferrable? 🙈

@micbar micbar mentioned this pull request May 24, 2022
25 tasks
@sonarcloud
Copy link

sonarcloud bot commented May 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit 655a1b5 into master May 24, 2022
@delete-merged-branch delete-merged-branch bot deleted the use-productversion-field branch May 24, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants