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

[OP#46179] create openproject group folder #364

Merged
merged 29 commits into from
Mar 23, 2023
Merged

Conversation

individual-it
Copy link
Collaborator

@individual-it individual-it commented Feb 20, 2023

@individual-it individual-it force-pushed the createGroupfolder branch 3 times, most recently from 8b2e355 to cc16b97 Compare March 3, 2023 07:17
@SwikritiT SwikritiT force-pushed the createGroupfolder branch from 4139876 to b8c7ac5 Compare March 6, 2023 11:03
@SwikritiT
Copy link
Contributor

SwikritiT commented Mar 7, 2023

App "Group folders" cannot be installed because it is not compatible with this version of the server.
https://github.com/nextcloud/integration_openproject/actions/runs/4352243691/jobs/7604789764#step:20:100

Similar behavior in my local machine too for the master of NC, maybe there's some regression in the master branch

@SwikritiT
Copy link
Contributor

nextcloud/groupfolders#2287 the tests should pass after the merge of this PR

@SwikritiT SwikritiT marked this pull request as ready for review March 9, 2023 05:00
@SwikritiT SwikritiT requested review from SagarGi and julien-nc March 9, 2023 05:00
@SwikritiT SwikritiT force-pushed the createGroupfolder branch 2 times, most recently from 97d31e0 to b30e65e Compare March 10, 2023 07:10
@SwikritiT
Copy link
Contributor

SwikritiT commented Mar 13, 2023

One problem with the current implementation is that if I send the POST request to the set-up endpoint with setup_group_folder = true when any openproject entities are present the nextcloud oauth credentials aren't created
Screenshot from 2023-03-13 14-39-23

This is because we create the oauth info

$status = $this->setIntegrationConfig($values);
$result = $this->recreateOauthClientInformation();

after we call the setIntegrationConfig, which throws an exception if any open project entities are present

@SwikritiT
Copy link
Contributor

SwikritiT commented Mar 13, 2023

One problem with the current implementation is that if I send the POST request to the set-up endpoint with setup_group_folder = true when any openproject entities are present the nextcloud oauth credentials aren't created Screenshot from 2023-03-13 14-39-23

This is because we create the oauth info

$status = $this->setIntegrationConfig($values);
$result = $this->recreateOauthClientInformation();

after we call the setIntegrationConfig, which throws an exception if any open project entities are present

One workaround for this can be 8dd8407

creating the NC oauth credentials when the OpenprojectGroupfolderSetupConflictException exception occurs as we know this exception is due to some configuration related to the OpenProject entities.

cc: @individual-it

lib/Service/OpenProjectAPIService.php Outdated Show resolved Hide resolved
lib/Service/OpenProjectAPIService.php Outdated Show resolved Hide resolved
@SwikritiT SwikritiT force-pushed the createGroupfolder branch 2 times, most recently from 16c5299 to 4269924 Compare March 16, 2023 07:20
individual-it and others added 13 commits March 20, 2023 09:42
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
SwikritiT and others added 13 commits March 20, 2023 09:42
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
@SwikritiT SwikritiT requested a review from SagarGi March 20, 2023 11:01
@SwikritiT
Copy link
Contributor

@individual-it @SagarGi this is ready for another round of review

Copy link
Collaborator Author

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

Looks very good to me, just some tiny comments

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
tests/lib/Service/OpenProjectAPIServiceTest.php Outdated Show resolved Hide resolved
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
Signed-off-by: Swikriti Tripathi <swikriti808@gmail.com>
@github-actions
Copy link

JS Code Coverage

Coverage after merging createGroupfolder into master will be
93.82%
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–27, 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
   utils.js57.45%33.33%50%59.52%10–14, 17–26, 6–9
src/components
   AdminSettings.vue99.24%95.92%82.35%99.86%1, 1, 1
   OAuthConnectButton.vue99.09%80%100%100%1
   PersonalSettings.vue98.88%91.67%83.33%100%1
src/components/admin
   FieldValue.vue95.56%62.50%100%98.77%1, 1, 1, 1
   FormHeading.vue98.98%75%100%100%1
   TextInput.vue98.46%80%87.50%100%1, 1, 1
src/components/icons
   ClippyIcon.vue93.18%50%50%97.50%1, 1
src/components/settings
   CheckBox.vue92.45%80%66.67%97.62%1, 1
   SettingsTitle.vue94.74%50%100%97.14%1, 1
src/components/tab
   EmptyContent.vue99.34%90.91%100%100%1
   SearchInput.vue99.56%83.33%100%100%1
   WorkPackage.vue99.01%33.33%100%99.66%1, 1, 1
src/utils
   workpackageHelper.js97.46%96%100%97.75%17–19
src/views
   Dashboard.vue98.36%50%50%99.66%1, 1, 1
   ProjectsTab.vue99.74%92.31%100%100%23

@github-actions github-actions bot deleted a comment from SwikritiT Mar 23, 2023
@github-actions
Copy link

PHP Code Coverage

Coverage after merging createGroupfolder into master will be
57.66%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
server/apps/integration_openproject/lib/AppInfo
   Application.php12.50%100%25%10.71%103–108, 119, 123, 65–66, 69, 73, 76, 82, 84–86, 88–90, 94–97, 99
server/apps/integration_openproject/lib/BackgroundJob
   RemoveExpiredDirectUploadTokens.php0%100%0%0%42, 44–46, 55–56
server/apps/integration_openproject/lib/Controller
   ConfigController.php63.25%100%50%64.09%131, 148–149, 151, 153–155, 157–158, 163–164, 166, 198–205, 332–333, 462–465, 467–468, 471, 479, 490, 504–506, 521–525, 527–528, 530–533, 535–537, 555–562, 564, 566, 569–571, 573–575, 589, 597–600, 602–605, 615–620
   DirectDownloadController.php0%100%0%0%36–38, 53–55, 57, 60–61
   DirectUploadController.php70.69%100%100%69.64%130–131, 176, 189, 193–196, 198, 208, 215, 229–231, 233–234, 237–239, 245, 247, 250, 252–253, 260–261, 264–265, 268–269, 289, 302, 307, 313
   FilesController.php73.74%100%100%72.04%168, 221–222, 267, 273–277, 281–283, 285, 287, 298–300, 303–304, 306–307, 311–314, 317
   OpenProjectAPIController.php85.07%100%78.57%85.56%136, 171, 188–189, 193, 197, 199–204, 206, 215–216, 219, 221, 223–226, 228–229, 234, 253, 278, 95
server/apps/integration_openproject/lib/Dashboard
   OpenProjectWidget.php0%100%0%0%101–102, 104–108, 116, 123–124, 126, 128–129, 131–132, 134, 137–138, 140–141, 143, 69–73, 80, 87, 94
server/apps/integration_openproject/lib/Exception
   OpenprojectErrorException.php100%100%100%100%
   OpenprojectFileNotUploadedException.php100%100%100%100%
   OpenprojectGroupfolderSetupConflictException.php0%100%0%0%20
   OpenprojectResponseException.php100%100%100%100%
server/apps/integration_openproject/lib/Listener
   BeforeGroupDeletedListener.php0%100%0%0%45, 53–54, 57–60
   BeforeNodeInsideOpenProjectGroupfilderChangedListener.php0%100%0%0%43–46, 50–55, 57–60, 63–65, 67, 69, 73
   BeforeUserDeletedListener.php0%100%0%0%45, 52–53, 55–58
   LoadSidebarScript.php0%100%0%0%101, 103–105, 107, 109, 111–112, 114–115, 117, 119, 72–78, 80–81, 83–84, 86–87, 93–94, 96–97, 99
   UserChangedListener.php0%100%0%0%49, 56–57, 60–64
server/apps/integration_openproject/lib/Migration
   Version2001Date20221213083550.php0%100%0%0%47, 57, 60, 63, 67, 70, 73, 77–79, 81
   Version2310Date20230116153411.php0%100%0%0%46, 49–52, 54–56, 60, 64, 68, 72, 76, 81–82, 84
server/apps/integration_openproject/lib/Search
   OpenProjectSearchProvider.php0%100%0%0%102, 109–110, 113–118, 120–121, 123–125, 128–129, 131–132, 136–141, 147–148, 150, 159–163, 171–179, 195–202, 211–216, 71–75, 82, 89, 97, 99
   OpenProjectSearchResultEntry.php100%100%100%100%
server/apps/integration_openproject/lib/Service
   DatabaseService.php43.90%100%60%41.67%125–128, 131, 80–87, 89–93, 95–97
   DirectDownloadService.php88%100%100%86.96%65–66, 68
   DirectUploadService.php54.55%100%66.67%52.63%112, 118, 79–82, 84, 89, 91
   OauthService.php0%100%0%0%37–38, 47–53, 55, 64–67, 78–85, 95–99
   OpenProjectAPIService.php75.85%100%78.79%75.57%158–162, 333–334, 336, 350–351, 360, 364, 385, 473–474, 481, 484–487, 489, 495, 499–501, 521, 530–533, 537–539, 544–546, 549–551, 553, 556–557, 560, 726, 797, 827–828, 830, 832–833, 856, 860, 864–865, 868–871, 873–874, 876–877, 879–880, 888–895, 901, 928, 930, 936, 941, 945, 951–953, 956, 958, 964, 993
server/apps/integration_openproject/lib/Settings
   Admin.php0%100%0%0%32–34, 41–43, 46–50, 53, 58–59, 62, 64–65, 67, 71, 75
   AdminSection.php0%100%0%0%19–20, 29, 39, 48, 55
   Personal.php87.88%100%50%93.10%94, 98
   PersonalSection.php0%100%0%0%19–20, 29, 39, 48, 55

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 👍

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.

3 participants