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

Merged
merged 5 commits into from
Mar 6, 2025

Conversation

nabim777
Copy link
Collaborator

@nabim777 nabim777 commented Feb 6, 2025

Description

In this PR, an error message is added which will display if the user_OIDC app is not supported with the integration OpenProject.

Related Issue or Workpackage

Screenshots (if appropriate):

image
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 4 times, most recently from 63061d9 to f704008 Compare February 10, 2025 07:20
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 2f23080 to c8d6120 Compare February 17, 2025 15:25
@nabim777 nabim777 marked this pull request as ready for review February 18, 2025 03:56
@nabim777 nabim777 self-assigned this Feb 18, 2025
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.)

@nabim777 nabim777 force-pushed the show-error-if-not-support branch 2 times, most recently from 3fb69cb to 2b7cbcc Compare March 3, 2025 09:44
@nabim777 nabim777 requested a review from individual-it March 3, 2025 09:45
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 1c72574 to 7651acb Compare March 3, 2025 11:01
@@ -86,11 +86,15 @@
<NcCheckboxRadioSwitch class="radio-check"
:checked.sync="authorizationMethod.authorizationMethodSet"
:value="authMethods.OIDC"
:disabled="!isOIDCAppInstalledAndEnabled"
:disabled="!isOIDCAppInstalledAndEnabled || !state.user_oidc_supported"
Copy link
Collaborator

Choose a reason for hiding this comment

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

for state.user_oidc_enabled we have a function isOIDCAppInstalledAndEnabled, so we should be consequent and also have one for state.user_oidc_supported and not using it directly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay @individual-it I will do that

<ErrorNote
v-if="isOIDCAppInstalledAndEnabled && !state.user_oidc_supported"
:error-title="messagesFmt.appNotSupported('user_oidc')"
:error-message="messagesFmt.minimumVersionRequired(state.user_oidc_minimum_version)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

use getters for the states as we do everywhere else

Copy link
Collaborator Author

@nabim777 nabim777 Mar 4, 2025

Choose a reason for hiding this comment

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

Okay I will follow that

@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 415717b to 2937663 Compare March 4, 2025 03:52
@nabim777 nabim777 requested a review from individual-it March 4, 2025 04:26
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 2937663 to 1cd4a5c Compare March 4, 2025 08:23
Copy link
Collaborator

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nabim777 nabim777 force-pushed the show-error-if-not-support branch 2 times, most recently from 5f5fd38 to d46c883 Compare March 4, 2025 08:33
@nabim777 nabim777 requested a review from saw-jan March 4, 2025 08:45
@nabim777 nabim777 force-pushed the show-error-if-not-support branch 2 times, most recently from 031f85f to a7132b0 Compare March 4, 2025 11:03
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from a7132b0 to fc55ba1 Compare March 5, 2025 11:20
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from fc55ba1 to 80be515 Compare March 5, 2025 11:26
@nabim777 nabim777 requested a review from individual-it March 5, 2025 11:28
@nabim777 nabim777 force-pushed the show-error-if-not-support branch from 80be515 to 23e8a9e Compare March 6, 2025 04:56
Copy link

github-actions bot commented Mar 6, 2025

JS Code Coverage

Coverage after merging show-error-if-not-support into master will be
90.12%
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.43%92.74%95.65%95.81%1010, 1021–1023, 1040–1042, 1054, 1054, 1054–1059, 1062–1063, 1067–1068, 1071–1072, 1076–1077, 1087–1092, 1189–1191, 1209–1221, 1236–1239, 1252–1255, 1276–1279, 1392, 1392–1393, 1393–1400, 1410–1411, 1471–1473, 1508–1511, 1547–1549, 1569–1572, 754–755, 861, 993–995
   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%
   ErrorNote.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%
   links.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, 556–560, 570–572, 591–593, 622–624, 646–648, 657–661
   Dashboard.vue92.96%92.86%82.61%93.77%120–125, 134, 144, 147, 158–160, 214–217, 220–221, 228–232, 67
   LinkMultipleFilesModal.vue99.14%97.56%100%99.32%157–159
   ProjectsTab.vue94.06%92.45%93.33%94.33%100–101, 107–109, 129, 140–141, 175–185, 234–236, 98–99

Copy link

github-actions bot commented Mar 6, 2025

PHP Code Coverage

Coverage after merging show-error-if-not-support into master will be
61.07%
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.php81.19%100%82.35%81.08%116, 155, 202–204, 207–214, 216–220, 222, 241, 266, 331, 381, 401, 448, 473–475, 478–482, 484, 75
   OpenProjectController.php96.45%100%80%96.95%241–245
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101, 108–109, 111–116, 118–122, 126–128, 131–142, 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.php28%100%40%26.67%100–105, 116–123, 72–80, 82–88, 97–99
   OpenProjectAPIService.php75.96%100%76.19%75.94%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,

@nabim777 nabim777 merged commit 56f92f3 into master Mar 6, 2025
20 checks passed
@nabim777 nabim777 deleted the show-error-if-not-support branch March 6, 2025 08:21
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.

3 participants