Skip to content

Comments

[SQSERVICES-1843] Outlook calendar feature flag#3025

Merged
battermann merged 6 commits intodevelopfrom
SQSERVICES-1843-be-oauth-feature-flag-rebase-develop
Jan 31, 2023
Merged

[SQSERVICES-1843] Outlook calendar feature flag#3025
battermann merged 6 commits intodevelopfrom
SQSERVICES-1843-be-oauth-feature-flag-rebase-develop

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Jan 26, 2023

https://wearezeta.atlassian.net/browse/SQSERVICES-1843

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 26, 2023
@battermann battermann requested a review from fisx January 26, 2023 11:59
@battermann battermann marked this pull request as ready for review January 26, 2023 12:00
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

2023-01-27-090100_1600x900_scrot

plus some nit-picks, and some bugs (i think).

(you can now just run ./services/start-services-only.sh and stern is already running. cool, didn't know that!)

Copy link
Contributor

Choose a reason for hiding this comment

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

Check lock status like in ExposeInvitationURLsToTeamAdminConfig above?

Copy link
Contributor Author

@battermann battermann Jan 27, 2023

Choose a reason for hiding this comment

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

I think, we don't want to check if the feature is locked here. This is an internal function and the necessary checks should already been made before this is called. And specifically for PATCH we want to allow overriding the team feature status even if locked because this is used by IBIS.

Copy link
Contributor

Choose a reason for hiding this comment

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

then why does the instance above do it? and why would ibis be allowed to break locks? ah, i see. it's locked for team admins, but ibis still needs to touch it.

oh dear what a mess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand why the function above does the check, either. I am currently trying to clarify this. Maybe it is a mistake.

It used to be much messier, believe me :)

@fisx
Copy link
Contributor

fisx commented Jan 27, 2023

I think the CI error can be ignored / fixed elsewhere. make ci package=galley passed on my machine.

@fisx
Copy link
Contributor

fisx commented Jan 27, 2023

anyway how does this flag work? what does it do? i suppose this depends on #2989 after all, so should be added in a third PR, maybe?

@battermann
Copy link
Contributor Author

I think the CI error can be ignored / fixed elsewhere. make ci package=galley passed on my machine.

Probably due to GHC upgrade, should work after develop is merged into this branch.

@battermann battermann requested a review from fisx January 27, 2023 12:04
@fisx
Copy link
Contributor

fisx commented Jan 27, 2023

could you reproduce the error in stern?

@battermann
Copy link
Contributor Author

battermann commented Jan 27, 2023

could you reproduce the error in stern?

Yes, IMO this has nothing to do with this PR and is a general problem.

The error occurs if the feature is locked.

If you unlock the feature with curl, then it works.

However, IIRC it used to be possible to lock or unlock features via backoffice/stern, but these endpoints have somehow disappeared. Strange.

I'd tend to solve this problem in a separate PR.

@battermann battermann force-pushed the SQSERVICES-1843-be-oauth-feature-flag-rebase-develop branch from e8b6c80 to ce44c0d Compare January 30, 2023 14:59
@battermann battermann merged commit 51a91bf into develop Jan 31, 2023
@battermann battermann deleted the SQSERVICES-1843-be-oauth-feature-flag-rebase-develop branch January 31, 2023 07:32

## Outlook calalendar integration

This feature setting only applies to the Outlook Calendar extension for Wire. As it is an external service, it should only be configured through this feature flag and otherwise ignored by the backend.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/As it is an external service, it should only be configured through this feature flag and otherwise ignored by the backend./This does not change the behavior of the backend, but it will be consulted by the [calendar integration service](TODO: which repo? ask dejan?), which will adopt its behavior accordingly./

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

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants