-
Notifications
You must be signed in to change notification settings - Fork 49
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
GO-4666: Contact book #1964
base: main
Are you sure you want to change the base?
GO-4666: Contact book #1964
Conversation
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
…o-4666-add-server-command-to-add-new-contact # Conflicts: # core/block/editor/spaceview.go # space/techspace/mock_techspace/mock_TechSpace.go
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
"github.com/anyproto/anytype-heart/pkg/lib/bundle" | ||
) | ||
|
||
var AllowedDetailsToChange = []domain.RelationKey{bundle.RelationKeyDescription} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make it a function to prevent the possibility to change it from other places in the app
if err != nil { | ||
return err | ||
} | ||
_, err = u.storeSource.PushStoreChange(ctx, source.PushStoreChangeParams{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should never trigger a push change, because otherwise whenever somebody changes something in their Identity we will add a new change
return nil | ||
} | ||
|
||
func (u *userDataObject) UpdateContactByIdentity(ctx context.Context, profile *model.IdentityProfile) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't need that method
return u.SmartBlock.Close() | ||
} | ||
|
||
func (u *userDataObject) SaveContact(ctx context.Context, profile *model.IdentityProfile, symKey crypto.SymKey) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be a model.IdentityProfile, but rather our custom data
} | ||
|
||
func (u *userDataObject) updateContactFields(profile *model.IdentityProfile, builder *storestate.Builder) error { | ||
contactId := domain.NewContactId(profile.Identity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't use identity profile here, it should be another model, which contains some fields (pls discuss with Yura), they shouldn't be taken from identity service
} | ||
|
||
func (u *userDataObject) createContactAndUpdateDetails(contact *Contact) { | ||
err := cache.DoContextFullID(u.objectCache, u.ctx, domain.FullID{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls move all this logic to a ContactService. This object should not now anything about how the contacts are being created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a responsibility of this object
} | ||
|
||
func (u *userDataObject) UpdateContactByDetails(ctx context.Context, contactId string, details *domain.Details) error { | ||
arena := u.arenaPool.Get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All stuff related to details should not be in this object
core/contact/service.go
Outdated
return s.registerExistingContacts(ctx) | ||
} | ||
|
||
func (s *service) registerExistingContacts(ctx context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a gist this is the service that should be responsible for managing the contact objects (i.e. smart blocks):
- update them
- create them
- delete them (remove from indexes and from cache)
onUpdateCallback func() | ||
} | ||
|
||
func New(sb smartblock.SmartBlock, crdtDb anystore.DB, objectCache cache.ObjectGetter) UserDataObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use the same crdt db as for the account object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because they both in tech space. Is it a problem?
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
Signed-off-by: AnastasiaShemyakinskaya <[email protected]>
https://linear.app/anytype/issue/GO-4666/add-server-command-to-add-new-contact