GO-3526 drag and drop files to collection add them to collection #1622
Conversation
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
core/block/editor/file/file_test.go
Outdated
| @@ -119,4 +140,122 @@ func TestDropFiles(t *testing.T) { | |||
| assert.Error(t, err) | |||
| assert.True(t, errors.Is(err, restriction.ErrRestricted)) | |||
| }) | |||
| t.Run("drop files in collection", func(t *testing.T) { | |||
There was a problem hiding this comment.
I don't understand what these tests verify
core/block/editor/file/file.go
Outdated
| if layout, ok := sf.Layout(); ok && layout == model.ObjectType_collection { | ||
| s := sf.NewState() | ||
| store := s.GetStoreSlice(template.CollectionStoreKey) | ||
| if !slices.Contains(store, info.file.TargetObjectId) { |
There was a problem hiding this comment.
It looks like the store already contains methods to verify if some object is already present.
https://en.wikipedia.org/wiki/Separation_of_concerns
core/block/editor/file/file.go
Outdated
| @@ -220,8 +222,10 @@ func (sf *sfile) updateFile(ctx session.Context, id, groupId string, apply func( | |||
| } | |||
|
|
|||
| func (sf *sfile) DropFiles(req pb.RpcFileDropRequest) (err error) { | |||
| if err = sf.Restrictions().Object.Check(model.Restrictions_Blocks); err != nil { | |||
| return err | |||
| if layout, ok := sf.Layout(); !ok || layout != model.ObjectType_collection { | |||
There was a problem hiding this comment.
please make a method called isCollection
There was a problem hiding this comment.
You can make it like so func isCollection(sb any), here you can check everything, is object a smartblock? does it have layout? is the layout collection?
core/block/editor/file/file.go
Outdated
| @@ -303,6 +306,14 @@ func (sf *sfile) dropFilesSetInfo(info dropFileInfo) (err error) { | |||
| s.Unlink(info.blockId) | |||
| return sf.Apply(s) | |||
| } | |||
| if layout, ok := sf.Layout(); ok && layout == model.ObjectType_collection { | |||
core/block/editor/file/file.go
Outdated
| @@ -476,6 +487,24 @@ func (dp *dropFilesProcess) Start(rootId, targetId string, pos model.BlockPositi | |||
| return | |||
| } | |||
| err = cache.Do(dp.picker, smartBlockIds[idx], func(sb File) error { | |||
| smartBlock, ok := sb.(smartblock.SmartBlock) | |||
| if ok { | |||
core/block/editor/file/file.go
Outdated
| @@ -476,6 +487,24 @@ func (dp *dropFilesProcess) Start(rootId, targetId string, pos model.BlockPositi | |||
| return | |||
| } | |||
| err = cache.Do(dp.picker, smartBlockIds[idx], func(sb File) error { | |||
| smartBlock, ok := sb.(smartblock.SmartBlock) | |||
There was a problem hiding this comment.
pls don't call everywhere smartblock[idx], flatEntries[idx], I know it was here before, but put them in a separate vars
core/block/editor/file/file.go
Outdated
| if layout, ok := smartBlock.Layout(); ok && layout == model.ObjectType_collection { | ||
| for _, entry := range flatEntries[idx] { | ||
| if entry.isDir { | ||
| smartBlockIds = append(smartBlockIds, rootId) |
There was a problem hiding this comment.
Why are we appending rootId? can we do it multiple times if we do it in a loop?
There was a problem hiding this comment.
If we do it only once we should structure the code in a way that it would be evident that this happens only once
There was a problem hiding this comment.
I would also extract this in a separate func
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
Signed-off-by: AnastasiaShemyakinskaya <shem98a@mail.ru>
AnastasiaShemyakinskaya
deleted the
go-3526-drag-and-drop-files-to-collection-add-them-to-collection
branch
https://linear.app/anytype/issue/GO-3526/drag-and-drop-files-to-collection-add-them-to-collection