tasking-manager icon indicating copy to clipboard operation
tasking-manager copied to clipboard

OSM Teams integration

Open willemarcel opened this issue 2 years ago • 18 comments

This PR allows users to get members and managers from an OSM Teams team.

How to test:

  • Create a new OAuth2 app on https://auth.mapping.team and copy the CLIENT_ID and CLIENT_SECRET. Use it to configure your TM instance, setting the following environment variables:
OSM_TEAMS_API_URL='https://dev.mapping.team' # it should use the dev instance now
OSM_TEAMS_CLIENT_ID=foo
OSM_TEAMS_CLIENT_SECRET=foo
  • Go to the team management page, login to OSM Teams, choose an OSM Teams team to sync from and create a new team.

Some screenshots

Team page

image

Team selection modal

image

Selected team details

image

Managers and members imported

image

Resync action button

image

willemarcel avatar Feb 09 '23 15:02 willemarcel

We noticed that adding a member to the team was possible via a join request. To avoid any unexpected issues, I think we should consider disabling all feature implementations related to modifying the members/managers list of a team that has been synced with OSM Teams. Since the sync is one-way, we’d want to ensure that any modifications to the members list for a team synced with OSM Teams are only done through the OSM Teams portal. Might as well create another team type for OSM Teams team on the backend.

I think it would be enough to switch the team to be "invite only" after the first osm teams sync, and disable the change later. What do you think?

Should we also update the user interface to add a visual clue for team managers indicating that modifying the members is only available through the OSM Teams portal?

I improved the text. Let me know if it is good that way.

Could you add this information to the user interface? Otherwise, it will be hard for managers to know who are the ones that weren’t imported.

I've added it too

Could you also add some info/docs on what the callback URL should be while creating an app on OSM Teams?

The callback url should be FRONTEND_URL + "/osmteams-authorized"

willemarcel avatar Mar 31 '23 19:03 willemarcel

I think it would be enough to switch the team to be "invite only" after the first osm teams sync, and disable the change later.

@HelNershingThapa this is done now.

willemarcel avatar Apr 05 '23 18:04 willemarcel

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 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Apr 05 '23 18:04 sonarqubecloud[bot]

@willemarcel - could you also add a field type in DB to track OSM teams? May be include one more option in join type. Since there is no way to send an invite via TM and the team management will entirely happen on OSM Teams platform, it is best we capture it here and accordingly control the options for the user. It is also easier for us to track the number of teams that are managed externally.

cc @HelNershingThapa @Aadesh-Baral

ramyaragupathy avatar May 02 '23 07:05 ramyaragupathy

@willemarcel, could you also add test cases for the changes made? We've been focusing on test coverage lately to ensure the code is as reliable and bug-free as possible. We'd love it if you added test cases for your PR to help us achieve our goal.

HelNershingThapa avatar May 02 '23 10:05 HelNershingThapa

@ramyaragupathy @HelNershingThapa I've added a new team join method named OSM_TEAMS. I've updated the UI of the team membership page with that new condition too. I also added the tests and rebased it with develop branch.

The build is failing in the CI, but I don't know the reason...

willemarcel avatar Jul 07 '23 18:07 willemarcel

The build is failing in the CI, but I don't know the reason...

It seems that the build failure might be connected to a potential conflict in the sequence of CSS bundle creation. I came across a relevant StackOverflow comment that discussed a similar problem. However, I currently don't have a specific solution or workaround to address the issue we're looking at.

HelNershingThapa avatar Jul 10 '23 11:07 HelNershingThapa

hi @willemarcel. Just checking in, have you been able to debug the issue?

HelNershingThapa avatar Jul 13 '23 14:07 HelNershingThapa

@HelNershingThapa The build works on my machine... Could you check if you can build it too?

willemarcel avatar Jul 13 '23 14:07 willemarcel

The build gets generated on my machine too; with the same warnings though.

HelNershingThapa avatar Jul 13 '23 15:07 HelNershingThapa

@eternaltyro anything that we have to configure in our CI setup? cc @willemarcel @HelNershingThapa

ramyaragupathy avatar Jul 14 '23 05:07 ramyaragupathy

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 4 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

sonarqubecloud[bot] avatar Jul 21 '23 17:07 sonarqubecloud[bot]

The build passed successfully upon removing the commit responsible for implementing lazy loading for routes in the try/5575-without-route-splitting branch. This has pointed toward the lazy loading implementation as a potential culprit behind the failure. We might need to investigate further into how the lazy loading implementation interacts with the build process, specifically the workaround commit https://github.com/hotosm/tasking-manager/pull/5575/commits/013dd364d8aee92e8cedaa54e66c707007329f84.

On another note, I attempted to increase the heap size incrementally from 2GB to 8GB using react-scripts --max_old_space_size=XXXX build, but unfortunately, this did not resolve the problem. The build continued to fail in the try/5575-osm-teams-circleci branch. So I think we can rule out the memory constraints as the root cause?

HelNershingThapa avatar Jul 27 '23 08:07 HelNershingThapa

The build passed successfully upon removing the commit responsible for implementing lazy loading for routes in the try/5575-without-route-splitting branch. This has pointed toward the lazy loading implementation as a potential culprit behind the failure. We might need to investigate further into how the lazy loading implementation interacts with the build process, specifically the workaround commit 013dd36.

In the past, I only used lazy loading in some components that had big size dependencies, like mapbox-gl and chartjs, some pages that were not used by most users, like the Management section, and some components used in many places, like the projects cards. Furthermore, as the application is cached by the service worker, the lazy loading is not helpful in most cases.

willemarcel avatar Aug 21 '23 19:08 willemarcel

@tsmock, could you offer some assistance here? You might have some insight into addressing the CSS issue we're encountering.

HelNershingThapa avatar Aug 22 '23 05:08 HelNershingThapa

Looking at https://app.circleci.com/pipelines/github/hotosm/tasking-manager?branch=try%2F5575-osm-teams-circleci (let me know if this is the wrong branch), it does look like an OOM issue in yarn build.

The "large" executors have 8g ram. I tested on a 4GB Raspberry Pi (nodejs 18, 8GB swapfile, node 18 with NODE_OPTIONS=--openssl-legacy-provider), and it succeeded. 4gb should be the default max_old_space_size value.

Using /usr/bin/time -f "%P %M" yarn build, the following data was obtained:

Type Time % CPU Memory (kb)
No polyfills, updated react-scripts dependencies 925s 142% 1938224kb
Polyfills, updated react-scripts dependencies 1339s 210% 1833852kb
base 1348s 198% 1807180kb
Reverted route splitting 543s 207% 1828924kb

Just to double-check the memory usage, I wrote a systemd unit for building the TM, as follows

# Put this in ${HOME}/.config/systemd/user/tasking-manager-build.service
[Unit]
Description=Build HOT TM with specific resource constraints

[Service]
Type=simple
# Point this at the frontend directory of the tm. %h is replaced by the user home directory.
WorkingDirectory=%h/workspace/tasking-manager/frontend

# Allows the build to work on Node 18, has to be commented out if using node 16
# Environment=NODE_OPTIONS=--openssl-legacy-provider
# Comment in/out depending upon whether or not you want to test with a specific node version
# Point it at the bin folder from a nodejs tarball
Environment=PATH=%h/node16/bin:/bin:/usr/bin
Environment=CI=true
Environment=GENERATE_SOURCEMAP=false

ExecStart=/usr/bin/yarnpkg build

MemoryAccounting=true
MemoryMax=2G
# Only needed to avoid having swap take care of higher usage than the max. This can be disabled if no swap is available.
MemorySwapMax=0


[Install]
WantedBy=default.target

Pre-revert of route splitting, the service got killed when it ran out of memory. Post-revert of route splitting, the service still got killed when it ran out of memory.

Increasing the MemoryMax to 4G resolved that issue. I have no clue why it is running out of memory on the CI.

For the CSS issue referenced, I'm looking into it.

tsmock avatar Aug 23 '23 17:08 tsmock

I've done a bit of investigating. Here are some possible solutions:

  1. Move all the import ".*.css"; into the index.html, index.js, or App.js
  2. Apply the following (note: I had reverted 013dd364d8aee92e8cedaa54e66c707007329f84, but I don't think that is necessary)
diff --git a/frontend/src/views/projectEdit.js b/frontend/src/views/projectEdit.js
index fb992c5ca..6ad3b9b27 100644
--- a/frontend/src/views/projectEdit.js
+++ b/frontend/src/views/projectEdit.js
@@ -6,10 +6,10 @@ import { FormattedMessage } from 'react-intl';
 
 import messages from './messages';
 import projectEditMessages from '../components/projectEdit/messages';
+import { PriorityAreasForm } from '../components/projectEdit/priorityAreasForm';
 import { DescriptionForm } from '../components/projectEdit/descriptionForm';
 import { InstructionsForm } from '../components/projectEdit/instructionsForm';
 import { MetadataForm } from '../components/projectEdit/metadataForm';
-import { PriorityAreasForm } from '../components/projectEdit/priorityAreasForm';
 import { ImageryForm } from '../components/projectEdit/imageryForm';
 import { PermissionsForm } from '../components/projectEdit/permissionsForm';
 import { SettingsForm } from '../components/projectEdit/settingsForm';

According to

tsmock avatar Aug 23 '23 18:08 tsmock

Having issue while trying to setup this PR locally

My local environment variables: OSM_TEAMS_AUTH_DOMAIN='https://auth.mapping.team' OSM_TEAMS_API_URL='https://mapping.team' OSM_TEAMS_CLIENT_ID=-- OSM_TEAMS_CLIENT_SECRET=-- TM_OSM_TEAMS_REDIRECT_URI=http://127.0.0.1:3000/osmteams-authorized

The ui shows for sync with osm teams while creating new team Screenshot 2024-03-28 101151

The authentication with auth.mapping.team works

Screenshot 2024-03-28 101320 Screenshot 2024-03-28 101352

But while trying to Select a team from osm teams it fails with unauthorized, invalid token There is osmteams_token in localstorage

Screenshot 2024-03-28 101443

On backend side the token are being fetched and on frontend stored on the use browser but the auth fails to https://mapping.team/api/my/teams

@willemarcel Can you provide me some insights what wrong over this.

kaditya97 avatar Mar 28 '24 04:03 kaditya97