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

CAN History Part 2 #3386

Merged
merged 31 commits into from
Feb 10, 2025
Merged

CAN History Part 2 #3386

merged 31 commits into from
Feb 10, 2025

Conversation

rajohnson90
Copy link
Contributor

@rajohnson90 rajohnson90 commented Jan 29, 2025

What changed

In this PR we have added the majority of the remaining CAN History events designed by UX.

The largest part of this change was the events for updates. Updates are handled by taking the model before and after the update, serializing them, and passing them to a utility function in the ops/utils/events.py. This utility function performs a diff of the root level properties on these objects and places them into a dictionary of changes along with a few other properties required by all update changes.

Once the changes have been collected by the utility function and passed to the OpsEvent, the other biggest change is the handling of updates in the can_messages.py file. For CAN Updates, we iterate over the list of keys in the change dictionary and create a CAN History message for each changed property that we have designed around. Primarily right now this is the nickname and description of a CAN, but also portfolio. Portfolio may be moved to a different function later as it seems like the only time we expect the managing portfolio for a CAN to change is during the yearly CANBACs import.
CAN Funding and CAN Budget only check for changes to the funding received and budget respectively.

Issue

OPS-3276

How to test

  • Run unit tests
  • Run E2E tests

Screenshots

If relevant, e.g. for a front-end feature

Definition of Done Checklist

  • OESA: Code refactored for clarity
  • OESA: Dependency rules followed
  • Automated unit tests updated and passed
  • Automated integration tests updated and passed
  • Automated quality tests updated and passed
  • Automated load tests updated and passed
  • Automated a11y tests updated and passed
  • Automated security tests updated and passed
  • 90%+ Code coverage achieved
  • Form validations updated

@rajohnson90 rajohnson90 requested a review from fpigeonjr February 4, 2025 16:03
@rajohnson90 rajohnson90 marked this pull request as ready for review February 4, 2025 16:05
Copy link
Contributor

@fpigeonjr fpigeonjr left a comment

Choose a reason for hiding this comment

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

🥇 looking good, tested locally and here is what I observed:

  • ✅ CAN budget added
  • ✅ CAN budget edited
  • ✅ CAN Funding Received added
  • ❌ CAN Funding Received edited
  • ✅ CAN Funding Received deleted
  • ✅ CAN Nickname edited (after small delay)
  • ✅ CAN Description edited (after small delay)

@fpigeonjr
Copy link
Contributor

these branch names remind how I used to name my photoshop files back in da day 😅
image

@fpigeonjr fpigeonjr requested a review from Santi-3rd February 4, 2025 23:37
@rajohnson90
Copy link
Contributor Author

@fpigeonjr Do you mean that editing funding received did not create an event in the history log?

@fpigeonjr
Copy link
Contributor

@fpigeonjr Do you mean that editing funding received did not create an event in the history log?

yes that is correct.

Copy link
Contributor

@fpigeonjr fpigeonjr left a comment

Choose a reason for hiding this comment

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

tested locally and works as expected

Copy link
Contributor

@johndeange johndeange 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! Great job @rajohnson90 and @Santi-3rd

@rajohnson90 rajohnson90 merged commit 02ff541 into main Feb 10, 2025
42 checks passed
@rajohnson90 rajohnson90 deleted the OPS-3276/can-history-pt-2-pt-final branch February 10, 2025 15:50
Copy link

🎉 This PR is included in version 1.41.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants