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

[WP #60683]display error if oidc apps not supported #768

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

nabim777
Copy link
Collaborator

@nabim777 nabim777 commented Feb 6, 2025

Description

Show the error if the user_OIDC app not supported to integration_openproject

Related Issue or Workpackage

Screenshots (if appropriate):

image

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 marked this pull request as draft February 6, 2025 04:56
@nabim777 nabim777 force-pushed the show-error-if-not-support branch 2 times, most recently from 3cc84d0 to 09cf76e Compare February 6, 2025 11:21
Signed-off-by: nabim777 <[email protected]>
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 09cf76e to a7d8d10 Compare February 6, 2025 11:25
Signed-off-by: nabim777 <[email protected]>
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 63061d9 to f704008 Compare February 10, 2025 07:20
Signed-off-by: nabim777 <[email protected]>
Signed-off-by: nabim777 <[email protected]>
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 2f23080 to c8d6120 Compare February 17, 2025 15:25
Copy link

JS Code Coverage

Coverage after merging show-error-if-not-support into master will be
89.26%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   adminSettings.js0%0%0%0%1, 1, 10–19, 2–9
   bootstrap.js0%0%0%0%1, 1, 10–12, 2–9
   dashboard.js0%0%0%0%1, 1, 10–19, 2–9
   fileActions.js0%0%0%0%1, 1, 10–19, 2, 20–23, 3–9
   personalSettings.js0%0%0%0%1, 1, 10–19, 2–9
   projectTab.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50, 6–9
   reference.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–45, 5–9
   utils.js77.27%33.33%50%79.52%12–20, 23–32
src/components
   AdminSettings.vue95.35%92.68%95.56%95.73%1008–1010, 1022, 1022, 1022–1027, 1030–1031, 1035–1036, 1039–1040, 1044–1045, 1055–1060, 1157–1159, 1177–1189, 1204–1207, 1220–1223, 1244–1247, 1360, 1360–1361, 1361–1368, 1378–1379, 1439–1441, 1476–1479, 1515–1517, 1537–1540, 728–729, 832, 961–963, 978, 989–991
   ErrorLabel.vue100%100%100%100%
   OAuthConnectButton.vue91.85%75%100%93.28%64–69, 72–76
   PersonalSettings.vue92.02%95.65%90%91.71%133–134, 144–149, 152–161
src/components/admin
   FieldValue.vue100%100%100%100%
   FormHeading.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.91%85.71%100%97.67%51–53
src/components/tab
   EmptyContent.vue96.45%80.95%100%98.24%102–105, 107–108, 97
   SearchInput.vue95.32%92.96%94.74%95.79%139–140, 193, 204–209, 268–270, 286–288, 292–297
   WorkPackage.vue86.25%73.17%93.33%87.62%107–116, 129–131, 142–146, 156–158, 176–182, 220, 220–225, 225, 225–236, 81–82
src/constants
   appID.js100%100%100%100%
   messages.js100%100%100%100%
src/filesPlugin
   filesPlugin.js0%0%0%0%1, 1, 10, 100, 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, 8–9
src/utils
   workpackageHelper.js93.80%93.10%88.89%94.24%100–102, 23–27, 54, 54–56, 97–99
src/views
   CreateWorkPackageModal.vue94.41%86.54%91.67%95.58%358–360, 363, 464–467, 472–477, 482–487, 493–496, 499, 515, 515, 555–559, 569–571, 590–592, 622–624, 646–648, 657–661
   Dashboard.vue79.54%80.70%68.18%80.13%108–114, 121–122, 126–131, 140, 150, 153, 156–159, 164–166, 207–213, 219–222, 224–234, 263–271, 284–298, 65, 79
   LinkMultipleFilesModal.vue99.14%97.56%100%99.32%157–159
   ProjectsTab.vue94.05%92.45%93.33%94.32%100–101, 107–109, 129, 140–141, 175–185, 233–235, 98–99

Copy link

PHP Code Coverage

Coverage after merging show-error-if-not-support into master will be
61.24%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib
   Capabilities.php0%100%0%0%24, 31–40
   ExchangedTokenRequestedEventHelper.php0%100%0%0%20, 27–29
   ServerVersionHelper.php0%100%0%0%23–24, 27, 30
server/apps/integration_openproject/lib/AppInfo
   Application.php7.41%100%25%6%100, 102–105, 107, 109, 111, 115–118, 120–131, 133, 136, 140, 144–146, 73–75, 78–82, 84–89, 91–92, 95
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%28, 30–32, 41–42
server/apps/integration_openproject/lib/Controller
   ConfigController.php69.96%100%55.56%70.55%132, 173–174, 176, 178–180, 182–185, 188–189, 191, 236, 246, 250–252, 355–359, 484–486, 488–490, 539, 627–630, 632–633, 636, 644–648, 659, 673–676, 684, 688–691, 705–709, 711–712, 714–730, 747–752, 754–755, 757–759, 762, 764–780, 793–800, 802–805, 807–811, 820–825
   DirectDownloadController.php0%100%0%0%33–35, 50–52, 54–61
   DirectUploadController.php70.83%100%100%70%124–126, 169–171, 182, 186–189, 191, 201, 208, 224–226, 228–229, 232–237, 240, 242, 250–252, 258–260, 268–270, 285–287, 306, 311, 317
   FilesController.php72.95%100%83.33%72.41%178–179, 241, 246–249, 251, 253–266, 269–270, 272–273, 277–280, 283, 289
   OpenProjectAPIController.php88.24%100%80.95%88.67%116, 157, 197, 245–247, 250–257, 259–263, 265, 284, 309, 376, 427, 448, 496, 522–524, 527–531, 533, 738–742
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108–115, 117–121, 125–126, 129–140, 61–66, 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%21
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%30, 38–39, 42–45, 47
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%46–48, 52–55, 57, 59, 62–63, 65, 67–70, 72–75, 77–79
   BeforeUserDeletedListener.php0%100%0%0%30, 37–38, 40–43, 45
   LoadAdditionalScriptsListener.php0%100%0%0%37–38, 46–47, 49, 51–52, 54–56, 58
   LoadSidebarScript.php0%100%0%0%100, 102–103, 105–106, 108, 110–111, 113, 115, 117–124, 128, 131–136, 70–88, 97–98
   OpenProjectReferenceListener.php0%100%0%0%44–46, 53–54, 56, 58–59, 61–73
   TermsOfServiceEventListener.php0%100%0%0%41–42, 47–48, 50–51, 53–55, 58–62
   UserChangedListener.php0%100%0%0%34, 41–42, 45–50, 53
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%30, 40–48, 50–58, 60–62, 64
   Version2310Date20230116153411.php0%100%0%0%29, 32–35, 37–62, 64–65, 67
   Version2400Date20230504144300.php0%100%0%0%30, 40–43
   Version2640Date20240628114301.php0%100%0%0%35, 47–49, 52–53, 56
server/apps/integration_openproject/lib/Reference
   WorkPackageReferenceProvider.php51.67%100%25%58.33%100–101, 104, 108, 142, 150–151, 159, 37, 44, 51, 58–60, 87, 93–96, 99
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%100–103, 105–108, 110–113, 117–118, 120–121, 124–133, 135–139, 52–55, 62, 69, 77, 79, 82, 89–90, 93–97, 99
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php42.31%100%60%40.43%108–112, 114, 63–76, 78–85
   DirectDownloadService.php88.46%100%100%87.50%62–63, 65
   DirectUploadService.php42.86%100%66.67%40%101, 62–65, 67–75, 95
   OauthService.php32.08%100%40%31.25%100–108, 119–126, 75–83, 85–91
   OpenProjectAPIService.php76.01%100%76.19%76%1001–1002, 1004, 1006–1007, 1030–1036, 1038–1045, 1047–1054, 1102–1104, 1106–1108, 1111–1115, 1117–1119, 1128–1131, 1134–1136, 1138, 1141–1146, 1150–1151, 1182–1185, 1204–1211, 1227–1228, 1235–1237, 1239–1242, 1246, 1255, 1273,

@nabim777 nabim777 marked this pull request as ready for review February 18, 2025 03:56
@nabim777 nabim777 self-assigned this Feb 18, 2025
}

public function isUserOIDCAppSupported(): bool {
$userOidcVersion = $this->appManager->getAppVersion('user_oidc');
Copy link
Collaborator

Choose a reason for hiding this comment

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

you check the version before checking that the app is installed. What will happen here if the app is not installed at all?

Comment on lines +4386 to +4397
if (count($classesExist) === 4 && array_product(array_map('class_exists', $classesExist))) {
$service = $this->getOpenProjectAPIServiceMock(
['isUserOIDCAppInstalledAndEnabled'],
[
'appManager' => $iAppManagerMock,
],
);
$service->method('isUserOIDCAppInstalledAndEnabled')->willReturn($installedAndEnabled);
$actualResult = $service->isUserOIDCAppSupported();
} else {
$actualResult = false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand that test. You only run the function when your provider has all classes in $classesExist, so what is the point in the test cases where a class is missing? You just set the result to false and check if it is false 😕

Copy link

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.

2 participants