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

Update event triggers for moped personnel and personnel roles #964

Merged
merged 14 commits into from Feb 2, 2023

Conversation

chiaberry
Copy link
Member

@chiaberry chiaberry commented Feb 1, 2023

Associated issues

cityofaustin/atd-data-tech#10810

Testing

URL to test:
https://deploy-preview-964--atd-moped-main.netlify.app/moped/projects

Steps to test:

  1. Add team members to a team
  2. Remove a team member
  3. Swap out one team member for a different name
  4. Do any other edits you can to a team member

Changing roles are tracked in the activity log, but aren't shown in the UI. You can check that the mutations are working locally by checking http://localhost:9695/console/events/data/activity_log_moped_proj_personnel_roles/processed or querying the db.

I also have a note about a bug I saw in staging where if you save empty spaces as a project description, you can't edit the description again (ex: https://moped.austinmobility.io/moped/projects/1790). Try saving a description as " " and you should see the "-" render so you can still edit the field.


Ship list

  • Code reviewed
  • Product manager approved
  • Added to QA test script if applicable

"record_type": {{ $body.table.name }},
"activity_id": {{ $body.id }},
"record_data": { "event": {{ $body.event }}},
"record_project_id": null,
Copy link
Member Author

Choose a reason for hiding this comment

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

moped_proj_personnel_roles does not have a column for the project_id, so this is null. This also means that while we are tracking these activities, they will not show up in our activity log query since we query on project_id

Copy link
Member

Choose a reason for hiding this comment

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

this is great—thanks for getting the tracking in place.

i also want to point out that, because we don't have the project ID here, we cannot update the project's modified date through this mutation. that's frustrating because i think user's should be able to expect that any project edit will update the project's modified date. if we wanted to get clever, we could mutate the moped_proj_personnel record from here, which would then fire off another event which would mutate the parent project's modified date. that would require us adding some mutable field on moped_proj_personnel like an updated_at timestamp.

just throwing it out there—i don't necessarily think this is a good idea. i believe we're also going to encounter this problem when we set up tracking for project features.

@@ -114,7 +114,7 @@ const ProjectSummaryProjectDescription = ({
)}
{!editMode && (
<ProjectSummaryLabel
text={description || " - "}
text={description.trim().length > 0 ? description : " - "}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to fix a bug I found where if you save empty spaces as a project description, you can't edit the description again (ex: https://moped.austinmobility.io/moped/projects/1790) Boolean(" ") is true.

Copy link
Member

Choose a reason for hiding this comment

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

thank you for fixing this!

we have a snackoo to prevent users from saving an empty description in the first place. we require description on project create but not project edit.

@@ -95,7 +95,6 @@ export const formatProjectActivity = (change, lookupList) => {
],
};
}
console.log(changeData.new[changedField])
Copy link
Member Author

Choose a reason for hiding this comment

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

😬

Copy link
Member

@johnclary johnclary left a comment

Choose a reason for hiding this comment

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

looks great and works great! since you're out-of-electricity i made a tiny patch to rework the edit event text.

🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢

"record_type": {{ $body.table.name }},
"activity_id": {{ $body.id }},
"record_data": { "event": {{ $body.event }}},
"record_project_id": null,
Copy link
Member

Choose a reason for hiding this comment

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

this is great—thanks for getting the tracking in place.

i also want to point out that, because we don't have the project ID here, we cannot update the project's modified date through this mutation. that's frustrating because i think user's should be able to expect that any project edit will update the project's modified date. if we wanted to get clever, we could mutate the moped_proj_personnel record from here, which would then fire off another event which would mutate the parent project's modified date. that would require us adding some mutable field on moped_proj_personnel like an updated_at timestamp.

just throwing it out there—i don't necessarily think this is a good idea. i believe we're also going to encounter this problem when we set up tracking for project features.

<Typography
variant="body2"
className={classes.entryText}
component="span"
Copy link
Member

Choose a reason for hiding this comment

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

this fixes an invalid DOM nesting error

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you!!!

@@ -114,7 +114,7 @@ const ProjectSummaryProjectDescription = ({
)}
{!editMode && (
<ProjectSummaryLabel
text={description || " - "}
text={description.trim().length > 0 ? description : " - "}
Copy link
Member

Choose a reason for hiding this comment

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

thank you for fixing this!

we have a snackoo to prevent users from saving an empty description in the first place. we require description on project create but not project edit.

Copy link
Contributor

@mddilley mddilley left a comment

Choose a reason for hiding this comment

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

Tried all things personnel, and it works like a dream! 🙌 Some cool bug fixes in here too. 👋 🐛 Seeing rich events like Replaced team member <team member> with <team member> really makes this activity log look so pro!!! 🚢

@chiaberry chiaberry merged commit d023f3e into main Feb 2, 2023
@johnclary johnclary deleted the 10810-personnel-activity branch February 16, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants