move goal feat #2067
move goal feat #2067
Conversation
…ide when a goal is moved into shared goal
… out of shared parent
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Looking good. Awesome to see you're tackling this complex issue! |
| {newParentTitle === "Non-shared goal" && ( | ||
| <div className="warning-message"> | ||
| <InfoCircleOutlined /> | ||
| <span>The new parent goal is not shared. Changes will be ignored if accepted.</span> |
There was a problem hiding this comment.
In my opinion, sharing should be per goal - independant of what the parent goal is.
If I/you change the parent goal of our shared goal - that should not affect propagation of changes on our shared goal, right?
There was a problem hiding this comment.
this is for the situation when child is moved out of the shared goal. the above code is stale.
If I/you change the parent goal of our shared goal - that should not affect propagation of changes on our shared goal, right?
this is handled differently. if we move the root shared alltogether then the suggestion will discarded altogether by the reciever.
There was a problem hiding this comment.
Yeah, what I'm saying is that maybe we should be dropping the whole concept of letting shared status depend on some 'root-goal'?
In my view, when you share a goal - all descendants are also shared (or not with opt-out).
From then on out - they are 'autonomous' from each other.
Of course if you delete a tree - all descendants will also be suggested to be deleted - even the ones your partner has moved since the sharing (since they are handled one-by-one). The ones you moved out of the tree will then not be deleted.
Basically, this makes sharing/deleting a tree the equivalent of individually sharing/deleting all goals in the tree. The advantage is that after that you are free to move things around individually without any extra complexity/dependency on other goals.
There was a problem hiding this comment.
I agree that dropping root-goal dependency and treating each goal independently would make the system more flexible and intuitive.
I'll start implementing these changes.
|
@tijlleenders when there is a change in shared descendant, where should the notification be displayed. root or the parent (if new goal added) / goal itself. |
|
@vinaybadgujar102 The idea is that the notification dot travels up the tree to the highest level - just like now. A possible difference with the current situation is that the dot should not care about sharing when 'travelling up' the tree. It is also realtime (not a property of ancestors) - as things are always moveable. Does this make sense? |
|
Great, some minor corrections:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -237,6 +299,23 @@ const DisplayChangesModal = ({ currentMainGoal }: { currentMainGoal: GoalItem }) | |||
| init(); | |||
| }, [currentMainGoal]); | |||
|
|
|||
| const getChangedGoalFromRoot = async (rootGoal: GoalItem, relId: string) => { | |||
There was a problem hiding this comment.
'getChangedGoalFromRoot' is assigned a value but never used.
|
@vinaybadgujar102 is this excluding LWW? |
I have implemented the function to handle the conflict resolution just have to integrate it with the existing code. |
|
Yes, please implement the sharing with LWW too, if it's ready to add. Currently, in this branch - if I rename a shared goal - it completely disappears from the other persons' partner page. The moving works fine, congrats! |
…xcept newgoalmoved and subgoals change)
|
@tijlleenders, integrated the LWW for collaborated goals for all actions excluding change of adding new subgoals to shared goals and movement of non shared goals to collaborated goals. Check if it is working correctly? Is there need to implement lww on partner goals? |
|
@vinaybadgujar102 This is AWESOME!!! |
Yeah. Sometimes only 2-3 starter goals are created after memory refresh. Its not introduced in this pr, I have been experiencing it from long time ago. Can be handled in another issue |
|
@Tushar-4781 Can you review the code? |
resolves #1918