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

Fix version comparison #723

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Fix version comparison #723

merged 2 commits into from
Oct 31, 2024

Conversation

nabim777
Copy link
Collaborator

@nabim777 nabim777 commented Oct 25, 2024

Description

The method getVersion() returns with 4 digit array Example: [27,0,0,7] which fails on the line https://github.com/nextcloud/integration_openproject/blob/master/lib/Service/OauthService.php#L62
This error is seen on nextcloud 27 version (i.e when variable $nextcloudversion is 27.0.0)
So, this PR is for fixing such error

Step to reproduce

  1. Clone the server of branch 27
git clone git@github.com:nextcloud/server.git -b stable27
  1. Go to path \server and Checkout to the v27.0.0
git checkout v27.0.0
  1. Set up nextcloud, enable integration_opneproject apps and openproject
  2. When we try to do oauth connection then the app crashes

Related Issue or Workpackage

Screen Record :

While oauth configuration on stable27 branch we got following error:
Screencast from 10-25-2024 02:29:03 PM.webm

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)
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Updated CHANGELOG.md file

@nabim777 nabim777 self-assigned this Oct 25, 2024
@nabim777 nabim777 marked this pull request as ready for review October 25, 2024 09:57
@nabim777 nabim777 marked this pull request as draft October 25, 2024 09:59
@nabim777 nabim777 marked this pull request as ready for review October 25, 2024 10:14
@nabim777 nabim777 force-pushed the fix/version-comparison branch 4 times, most recently from 577ac6a to a672ead Compare October 28, 2024 04:17
@nabim777 nabim777 changed the title Fix version comparison for stable27 Fix version comparison Oct 28, 2024
@nabim777 nabim777 requested a review from SagarGi October 28, 2024 04:18
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the fix/version-comparison branch from a672ead to 3776d6a Compare October 28, 2024 06:26
case version_compare($nextcloudVersion, '30.0.0.0') >= 0:
case version_compare($nextcloudVersion, '29.0.7.0') >= 0 && version_compare($nextcloudVersion, '30.0.0.0') < 0:
case version_compare($nextcloudVersion, '28.0.10.0') >= 0 && version_compare($nextcloudVersion, '29.0.0.0') < 0:
case version_compare($nextcloudVersion, '27.1.11.8') >= 0 && version_compare($nextcloudVersion, '28.0.0.0') < 0:
Copy link
Collaborator

@SagarGi SagarGi Oct 28, 2024

Choose a reason for hiding this comment

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

@julien-nc has provided some specific nextcloud version with minipatch version as well like 27.1.11.8 in work package https://community.openproject.org/projects/nextcloud-integration/work_packages/57654/activity. I think we need to be sure that the hash encryption (new change in Nextcloud) is included for all NC versions that is listed on work packages. For example in the work package its says hash is included in 30. does it mean 30.0.0.0 or 30.0.0.2 or 30.0.0.3? It is not very specific. I think may be we need to be very specific at this case for this code? Adding zero may be won't work?

CC @individual-it @julien-nc

Copy link
Collaborator

Choose a reason for hiding this comment

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

good point, please find out the micro versions

Copy link
Member

Choose a reason for hiding this comment

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

  • Secrets are hashed in all v30.x . The v30 release was actually 30.0.0.14
  • Secrets are hashed in v29.0.7 and higher. The v29.0.7 release was 29.0.7.1
  • Secrets are hashed in v28.0.10 and higher. The v28.0.10 release was 28.0.10.1
  • ⚠️ Secrets are hashed in v27.1.11.9 and higher. The v27.1.11 release was actually 27.1.11.3 and still encrypts (does not hash yet). The enterprise release 27.1.11.9 hashes the secrets. Micro version is indeed important for 27.

@nabim777 nabim777 force-pushed the fix/version-comparison branch from c4c04d3 to ba20b94 Compare October 31, 2024 10:33
Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
@nabim777 nabim777 force-pushed the fix/version-comparison branch from ba20b94 to 748825f Compare October 31, 2024 10:50
Copy link

JS Code Coverage

Coverage after merging fix/version-comparison into master will be
87.67%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   adminSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   bootstrap.js0%0%0%0%1, 1–7
   dashboard.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   fileActions.js0%0%0%0%1, 1, 10–17, 2–9
   personalSettings.js0%0%0%0%1, 1, 10–19, 2, 20–25, 3–9
   projectTab.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–66, 7–9
   reference.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60, 7–9
   utils.js71.43%33.33%50%73.85%10–14, 17–26, 6–9
src/components
   AdminSettings.vue96.16%93.83%95.16%96.52%1033–1036, 1072–1074, 1094–1097, 518–519, 655, 667–669, 681, 681, 681–686, 689–690, 694–695, 698–699, 703–704, 714–719, 779–781, 793–796, 809–811, 996–998
   OAuthConnectButton.vue85.27%63.64%100%86.84%44–51, 59–64, 67–71
   PersonalSettings.vue90.16%94.44%85.71%89.88%100, 110–115, 118–127, 99
src/components/admin
   FieldValue.vue100%100%100%100%
   FormHeading.vue100%100%100%100%
   ProjectFolderError.vue100%100%100%100%
   TermsOfServiceUnsigned.vue100%100%100%100%
   TextInput.vue100%100%100%100%
src/components/icons
   ClippyIcon.vue100%100%100%100%
   OpenProjectIcon.vue100%100%100%100%
src/components/settings
   CheckBox.vue100%100%100%100%
   SettingsTitle.vue96.74%85.71%100%97.53%46–48
src/components/tab
   EmptyContent.vue96.65%83.33%100%98.06%87–90, 92–93
   SearchInput.vue95.27%92.96%94.74%95.73%134–135, 188, 199–204, 263–265, 281–283, 287–292
   WorkPackage.vue86.10%73.17%93.33%87.46%102–111, 124–126, 137–141, 151–153, 171–177, 215, 215–220, 220, 220–231, 76–77
src/filesPlugin
   filesPlugin.js0%0%0%0%1, 1, 10, 100–104, 11–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–79, 8, 80–89, 9, 90–99
   filesPluginLessThan28.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–78, 8–9
src/utils
   workpackageHelper.js93.73%93.10%88.89%94.15%18–22, 49, 49–51, 94–99
src/views
   CreateWorkPackageModal.vue94.34%86.54%91.67%95.50%353–355, 358, 459–462, 467–472, 477–482, 488–491, 494, 510, 510, 550–554, 564–566, 585–587, 617–619, 641–643, 652–656
   Dashboard.vue77.40%80.39%61.90%78.01%103–108, 115, 119–120, 125, 128, 131–134, 139–141, 182–188, 194–197, 199–209, 238–246, 259–273, 51, 63, 88–91, 98–99
   LinkMultipleFilesModal.vue100%100%100%100%
   ProjectsTab.vue94.91%94.23%93.33%95.07%113–116, 142, 153–154, 188–198, 246–248

Copy link

PHP Code Coverage

Coverage after merging fix/version-comparison into master will be
60.63%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib
   Capabilities.php0%100%0%0%19, 26–35
   ServerVersionHelper.php0%100%0%0%17–18, 21, 24
server/apps/integration_openproject/lib/AppInfo
   Application.php7.41%100%25%6%101, 103–106, 108, 110, 112, 116–119, 121–132, 134, 137, 141, 145–147, 74–76, 79–83, 85–90, 92–93, 96
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php64.76%100%50%65.37%135, 152–153, 155, 157–159, 161–164, 167–168, 170, 212, 222, 226–228, 292–296, 406–408, 410–412, 417–420, 461, 549–552, 554–555, 558, 566–570, 581, 595–598, 606–607, 611–614, 628–632, 634–635, 637–653, 670–675, 677–678, 680–682, 685, 687–703, 716–723, 725–728, 730–734, 743–748
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57–64
   DirectUploadController.php70.83%100%100%70%141–143, 186–188, 199, 203–206, 208, 218, 225, 241–243, 245–246, 249–254, 257, 259, 267–269, 275–277, 285–287, 302–304, 323, 328, 334
   FilesController.php72.95%100%83.33%72.41%181–182, 244, 249–252, 254, 256–269, 272–273, 275–276, 280–283, 286, 292
   OpenProjectAPIController.php85.60%100%80%85.92%139, 180, 204, 230–233, 236–243, 245–249, 251, 263, 272, 290, 299, 369, 371, 421, 423, 443, 445, 492, 494, 520–523, 526–530, 532, 737–741, 96
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108, 115–116, 118, 120–137, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectAvatarErrorException.php100%100%100%100%
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectGroupfolderSetupConflictException.php100%100%100%100%
   OpenprojectResponseException.php100%100%100%100%
   OpenprojectUnauthorizedUserException.php0%100%0%0%16
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%48, 56–57, 60–63, 65
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%41–43, 47–50, 52, 54, 57–58, 60, 62–65, 67–70, 72–74
   BeforeUserDeletedListener.php0%100%0%0%48, 55–56, 58–61, 63
   LoadAdditionalScriptsListener.php0%100%0%0%18–19, 21–23, 25
   LoadSidebarScript.php0%100%0%0%100, 102, 104–105, 107, 109, 111, 113–122, 75–91, 96–97, 99
   OpenProjectReferenceListener.php0%100%0%0%53–54, 57–58, 61–62, 64–67
   TermsOfServiceEventListener.php0%100%0%0%59–60, 65–66, 68–69, 71–73, 76–80
   UserChangedListener.php0%100%0%0%52, 59–60, 63–68, 71
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57–65, 67–75, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–79, 81–82, 84
   Version2400Date20230504144300.php0%100%0%0%47, 57–60
   Version2640Date20240628114301.php0%100%0%0%52, 64–66, 69–70, 73
server/apps/integration_openproject/lib/Reference
   WorkPackageReferenceProvider.php51.67%100%25%58.33%102, 108–111, 114–116, 119, 123, 157, 165–166, 174, 52, 59, 66, 73–75
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%103–104, 107–118, 121–122, 124–125, 128–137, 139–143, 66–69, 76, 83, 91, 93, 96
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php42.31%100%60%40.43%100–102, 125–129, 131, 80–93, 95–99
   DirectDownloadService.php88.46%100%100%87.50%65–66, 68
   DirectUploadService.php42.86%100%66.67%40%112, 118, 79–82, 84–92
   OauthService.php32.08%100%40%31.25%103–111, 122–129, 78–86, 88–94
   OpenProjectAPIService.php75.63%100%75.93%75.60%1035–1037, 1039–1041, 1044–1048, 1050–1052, 1061–1064, 1067–1069, 1071, 1074–1079, 1083–1084, 1115–1118, 1137–1144, 1153–1154, 1161–1163, 1165–1168, 1172, 1181, 1199, 1201–1204, 1206–1211, 1353, 1365, 1368, 1390, 1393, 1403–1408, 1533–1535, 1537–1538, 1542–1543, 1545, 1547,

Copy link
Collaborator

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@SagarGi SagarGi merged commit 1bd13c6 into master Oct 31, 2024
17 checks passed
nabim777 added a commit that referenced this pull request Oct 31, 2024
* Fix version comparison for stable27

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

* mini patch updated

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>

---------

Signed-off-by: nabim777 <nabinalemagar019@gmail.com>
individual-it added a commit that referenced this pull request Oct 31, 2024
@nabim777 nabim777 deleted the fix/version-comparison branch November 1, 2024 08:22
Copy link

github-actions bot commented Nov 9, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants