Skip to content

Comments

(Un-)lock Status for Self-Deleting Messages#1937

Merged
fisx merged 31 commits intodevelopfrom
SQSERVICES-960-distinguish-between-unlock-team-feature-and-configure-team-feature
Nov 25, 2021
Merged

(Un-)lock Status for Self-Deleting Messages#1937
fisx merged 31 commits intodevelopfrom
SQSERVICES-960-distinguish-between-unlock-team-feature-and-configure-team-feature

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Nov 23, 2021

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

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If HTTP endpoint paths have been added or renamed, the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
  • If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
    • If new config options introduced: added usage description under docs/reference/config-options.md
    • If new config options introduced: recommended measures to be taken by on-premise instance operators.
    • If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.
    • If a data migration (not schema migration) introduced: measures to be taken by on-premise instance operators.
    • If public end-points have been changed or added: does nginz need un upgrade?
    • If internal end-points have been added or changed: which services have to be deployed in a specific order?

battermann and others added 10 commits November 22, 2021 08:20
json roundtrip test for PaymentStatus

C* schema migration for self deleting messages payment status

teamstore implementation for self deleting messages payment status

removed unnecessary function

dynamic lock/unlock payment status

added payment status to feature with config models

removed redundant description

use PaymentStatusValue for API path

removed redundant comment

added roundtrip test for PaymentStatusValue

updated API description

make payment status in config optional

golden tests include payment status, formatting

generic set payment status

error for locked payment status

validate payment status while setting team feature status

fix integration tests

comments
* Fix swagger-1.2.

* Simplify.
Co-authored-by: fisx <mf@zerobuzz.net>
@battermann battermann requested a review from fisx November 23, 2021 11:37
@fisx fisx changed the title (Un-) Lock Payment Status for Selfdeleting Messages (Un-)lock Payment Status for Selfdeleting Messages [skip ci] Nov 23, 2021
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.

Almost there!

I am wondering though if we want to take the "payment" out of all names again. If we have a feature where we want to make the site config overrule the team config, it may make sense to "lock" it when setting a site-wide config, and we may want to use the same thing there as we use here.

Let's discuss this!

battermann and others added 9 commits November 24, 2021 11:36
…and-configure-team-feature' of github.com:wireapp/wire-server into SQSERVICES-960-distinguish-between-unlock-team-feature-and-configure-team-feature
Co-authored-by: fisx <mf@zerobuzz.net>
Co-authored-by: fisx <mf@zerobuzz.net>
Co-authored-by: fisx <mf@zerobuzz.net>
Co-authored-by: fisx <mf@zerobuzz.net>
Co-authored-by: fisx <mf@zerobuzz.net>
@battermann battermann changed the title (Un-)lock Payment Status for Selfdeleting Messages [skip ci] (Un-)lock Payment Status for Selfdeleting Messages Nov 24, 2021
…and-configure-team-feature' of github.com:wireapp/wire-server into SQSERVICES-960-distinguish-between-unlock-team-feature-and-configure-team-feature
@battermann battermann requested a review from fisx November 24, 2021 16:10
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.

Now I want to change everything so it doesn't say "payment status" but "lock status". Otherwise I'm happy with everything now, or will be happy to change it in later PRs.

ld-options: -static
default-language: Haskell2010

-- FUTUREWORK(Leif): rename to galley-tests here and in the package.yaml file
Copy link
Contributor

Choose a reason for hiding this comment

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

#1942 :) (if you approve that PR, please merge right away.)

Public.defaultSelfDeletingMessagesStatus {Public.tfwcapsPaymentStatus = Public.PaymentUnlocked}
_ -> Public.defaultSelfDeletingMessagesStatus
(Just Public.PaymentLocked, _) -> Public.defaultSelfDeletingMessagesStatus
(Nothing, _) -> Public.defaultSelfDeletingMessagesStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

right, i missed the Nothing. makes it slightly less pretty, but i still prefer the way it reads now. thanks!

(it turns out there is more than one reason to lock a feature from
configuration, eg., because it is configured for the site and
shouldn't be changed by team admins.)
@fisx fisx changed the title (Un-)lock Payment Status for Selfdeleting Messages (Un-)lock Status for Self-Deleting Messages Nov 24, 2021
@fisx
Copy link
Contributor

fisx commented Nov 24, 2021

If HTTP endpoint paths have been added or renamed, the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.

Actually, we should grant QA access on staging to the new internal end-point. This is documented in the wiki. I'll push another commit.

If new config options introduced: added usage description under docs/reference/config-options.md

n/a

If new config options introduced: recommended measures to be taken by on-premise instance operators.

n/a

If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.

n/a (our migration is backwards compatible, meaning we can run old galley instances on the new schema, it'll just ignore the new column).

If a data migration (not schema migration) introduced: measures to be taken by on-premise instance operators.

n/a (data migration would be eg. "insert 1 into all rows into the new column")

If public end-points have been changed or added: does nginz need un upgrade?

n/a

If internal end-points have been added or changed: which services have to be deployed in a specific order?

no specific order. however, ibis will start using this end-point eventually, but only after we're done deploying. ibis should contain a note in the release notes when it starts using this end-point that it needs galley or newer.

@fisx
Copy link
Contributor

fisx commented Nov 24, 2021

Actually, we should grant QA access on staging to the new internal end-point. This is documented in the wiki. I'll push another commit.

correction: I'll make another (private) PR: https://github.com/zinfra/cailleach/pull/734

@fisx
Copy link
Contributor

fisx commented Nov 25, 2021

integration tests pass locally.

@fisx fisx merged commit 39f45ff into develop Nov 25, 2021
@fisx fisx deleted the SQSERVICES-960-distinguish-between-unlock-team-feature-and-configure-team-feature branch November 25, 2021 19:22
fisx added a commit that referenced this pull request Nov 26, 2021
fisx added a commit that referenced this pull request Nov 26, 2021
fisx pushed a commit that referenced this pull request Dec 3, 2021
battermann added a commit that referenced this pull request Dec 8, 2021
(Un-)lock Status for Self-Deleting Messages (#1937)

Co-authored-by: fisx <mf@zerobuzz.net>
Co-authored-by: Leif Battermann <leif.battermann@wire.com>
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.

2 participants