Skip to content

Comments

Lock/Unlock Payment Status for Self Deleting Messages#1916

Closed
battermann wants to merge 10 commits intodevelopfrom
SQSERVICES-574-self-deleting-messages-config-unlock-payment
Closed

Lock/Unlock Payment Status for Self Deleting Messages#1916
battermann wants to merge 10 commits intodevelopfrom
SQSERVICES-574-self-deleting-messages-config-unlock-payment

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Nov 9, 2021 •
edited
Loading

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:
    • 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?

fisx
fisx reviewed Nov 9, 2021
View reviewed changes
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.

looks good so far!

@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch 6 times, most recently from 8a62eaf to f9a1cc6 Compare November 11, 2021 13:42
@battermann battermann changed the title Unlock Payment Status for Self Deleting Messages Lock/Unlock Payment Status for Self Deleting Messages Nov 11, 2021
@battermann battermann requested a review from fisx November 11, 2021 16:08
@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch from 759f799 to 88cceb5 Compare November 12, 2021 09:37
fisx
fisx requested changes Nov 12, 2021
View reviewed changes
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.

Looks good, but of course I have comments. :)

@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch 2 times, most recently from e9abb7d to 8189709 Compare November 15, 2021 11:58
fisx
fisx reviewed Nov 15, 2021
View reviewed changes
@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch 2 times, most recently from 214f38d to 515a31c Compare November 17, 2021 09:32
@battermann battermann requested a review from fisx November 17, 2021 09:35
fisx
fisx requested changes Nov 17, 2021
View reviewed changes
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.

tests missing, and some more changes, but you're getting there! :)

services/galley/src/Galley/Cassandra/TeamFeatures.hs
m (Maybe PaymentStatus)
getPaymentStatus _ tid =
case paymentStatusCol @a of
Nothing -> pure Nothing
Copy link
Contributor

@fisx fisx Nov 17, 2021

Choose a reason for hiding this comment

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

I would have expected unlocked here instead of nothing, but you can also reasonably decide to handle this further up the call tree.

Copy link
Contributor Author

@battermann battermann Nov 18, 2021

Choose a reason for hiding this comment

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

Yes, it was my intention to push this to the caller.

@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch from f6d45b4 to d42e72d Compare November 18, 2021 13:08
@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch 2 times, most recently from b56bd71 to 4e9723f Compare November 19, 2021 13:34
battermann pushed a commit that referenced this pull request Nov 19, 2021
@fisx @battermann
This is a delta for #1916 (#1935)
816ec69 
* Fix swagger-1.2.

* Simplify.
@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch from 4e9723f to 29b5ae4 Compare November 19, 2021 13:38
battermann and others added 6 commits November 22, 2021 08:20
@battermann
Unlock payment route and functionality, no op cassandra impl yet
c199e00 
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
@battermann
removed payment status for app lock team feature from C* schema, and …
a87fe0c 
…updated C* interpreter
@fisx @battermann
This is a delta for #1916 (#1935)
9a09469 
* Fix swagger-1.2.

* Simplify.
@fisx @battermann
improve comment
9982454 
Co-authored-by: fisx <mf@zerobuzz.net>
@battermann
simplification
0a51b92
@battermann
require HasPaymentStatusCol for setting the payment status
f0f50cc
@battermann battermann force-pushed the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch from 29b5ae4 to 9d419ba Compare November 22, 2021 22:48
@battermann battermann marked this pull request as ready for review November 22, 2021 22:56
@battermann battermann requested a review from fisx November 22, 2021 22:56
@battermann battermann closed this Nov 23, 2021
@battermann battermann deleted the SQSERVICES-574-self-deleting-messages-config-unlock-payment branch November 23, 2021 11:35
@fisx
Copy link
Contributor

fisx commented Nov 23, 2021

obsoleted by #1937

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

Reviewers

@fisx fisx Awaiting requested review from fisx

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@battermann @fisx