Skip to content
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

Be able to edit scheduleEntries from activity and to edit activityResponsibles #4375

Conversation

manuelmeister
Copy link
Member

@manuelmeister manuelmeister commented Jan 2, 2024

It makes sense to be able to edit the activity properties from the activity view.
Bildschirmfoto 2024-01-02 um 23 56 11
(I thought it might be better to also be able to also be able to edit the title and category, to help people that did not get the trick to click on the title / category to change it.)

Also it makes sense to be able to edit the activity responsibles in the edit dialog.
Bildschirmfoto 2024-01-02 um 23 55 48

This is restricted to the edit dialog and is not implemented for the creation dialog.

@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label Jan 2, 2024
Copy link

github-actions bot commented Jan 2, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

@manuelmeister manuelmeister marked this pull request as ready for review January 2, 2024 20:31
@manuelmeister manuelmeister added UX/UI External feedback This Issue is Based on Feedback from the Community labels Jan 2, 2024
@manuelmeister
Copy link
Member Author

@carlobeltrame moved it to the right side.
Bildschirmfoto 2024-01-06 um 18 44 17

Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

Just as info:
There was once another proposal to add this feature: #1379

</tbody>
</table>
<DialogActivityEdit
v-if="activity"
Copy link
Contributor

Choose a reason for hiding this comment

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

you should hide or disable the button for guests. (if the activity is not editable)

Comment on lines +144 to +173
if (!this.hideHeaderFields) {
const untouchedActivityResponsibles = intersectionWith(
this.initialResponsibles,
this.entityData.activityResponsibles,
(a, b) => {
return a.campCollaboration._meta.self === b.campCollaboration._meta.self
}
)

const differentActivityResponsibles = differenceWith(
[...this.initialResponsibles, ...this.entityData.activityResponsibles],
untouchedActivityResponsibles,
(a, b) => {
return a.campCollaboration._meta.self === b.campCollaboration._meta.self
}
)

differentActivityResponsibles.forEach((activityResponsible) => {
if (activityResponsible?._meta?.self) {
promises.push(this.api.del(activityResponsible._meta.self))
} else {
promises.push(
this.activityResponsibles.$post({
activity: this.currentActivity._meta.self,
campCollaboration: activityResponsible.campCollaboration._meta.self,
})
)
}
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is solved little to simple. (even though i see a lot of thought process)

  1. This should not be in the update logic of the scheduleEntries
  2. In this function all the promises to update things (scheduleEntries, the activity itself) are collected and awaited. This should be preserved.

I would split the 2 changes in separate PR and first add the possibility to edit the scheduleEntries in the Activity View.
And then add the responsibles to the activty dialog in the picasso.

Copy link
Member Author

Choose a reason for hiding this comment

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

@BacLuc "too simple"? What should I make more complicated/different?

  1. This should not be in the update logic of the scheduleEntries

Why?

  1. … This should be preserved.

It is, it just adds the update scheduleEntries to the same promises array as scheduleEntries and activity.

Copy link
Contributor

Choose a reason for hiding this comment

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

@BacLuc "too simple"? What should I make more complicated/different?

  1. This should not be in the update logic of the scheduleEntries

Why?

But you don't want to send the post/delete requests for the activityResponsibles for each ScheduleEntry.
(The delete and posts for the same insertion/deletion are sent for each scheduleEntry once as i read the code).

  1. … This should be preserved.

It is, it just adds the update scheduleEntries to the same promises array as scheduleEntries and activity.

but something seems to not work:
Screencast from 07.01.2024 14:24:44.webm

Copy link
Contributor

Choose a reason for hiding this comment

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

uups, sorry.
Its not in the update for the scheduleentries.

@manuelmeister
Copy link
Member Author

@BacLuc hmm, do you know why this was not merged?

@BacLuc
Copy link
Contributor

BacLuc commented Jan 7, 2024

@BacLuc hmm, do you know why this was not merged?

I don't know it, but i vaguely remember that this was at the time we switched to api-platform.
So maybe we put it back in favor of more important things, and then it was so old we closed it
and did not start again until now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR External feedback This Issue is Based on Feedback from the Community UX/UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants