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

[To Main] Task - DESENG-676: Combine custom and summary content #2580

Merged
merged 12 commits into from
Sep 3, 2024

Conversation

NatSquared
Copy link
Contributor

Issue #: 🎟️ DESENG-676
Description of changes:

  • Task Merge engagement summary content and custom content
    • Combine the text and rich content fields from redundant types SummaryContent and CustomContent and store them in
      the existing EngagementContent table
    • Update the API to handle the new content structure
    • Todo: Update the frontend to properly leverage the new content structure and allow editing of the simplified content type
      User Guide update ticket (if applicable):

@NatSquared NatSquared changed the title Task/deseng 676 combine custom and summary content [To Main] Task - DESENG-676: Combine custom and summary content Aug 23, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 89.70588% with 7 lines in your changes missing coverage. Please review.

Project coverage is 76.02%. Comparing base (b0cb3b1) to head (a3a5461).

Files with missing lines Patch % Lines
...ngagementFormTabs/EngagementContent/TabContent.tsx 55.55% 4 Missing ⚠️
...gagementFormTabs/EngagementContent/ContentTabs.tsx 84.61% 2 Missing ⚠️
...et-api/src/met_api/resources/engagement_content.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2580      +/-   ##
==========================================
- Coverage   76.03%   76.02%   -0.02%     
==========================================
  Files         609      597      -12     
  Lines       22105    21652     -453     
  Branches     1797     1770      -27     
==========================================
- Hits        16808    16460     -348     
+ Misses       5033     4936      -97     
+ Partials      264      256       -8     
Flag Coverage Δ
metapi 87.45% <95.83%> (-0.18%) ⬇️
metweb 65.28% <86.36%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
met-api/src/met_api/models/__init__.py 100.00% <ø> (ø)
met-api/src/met_api/models/engagement_content.py 95.00% <100.00%> (-0.13%) ⬇️
met-api/src/met_api/resources/__init__.py 100.00% <ø> (ø)
...et_api/resources/engagement_content_translation.py 76.82% <100.00%> (ø)
met-api/src/met_api/schemas/engagement_content.py 100.00% <100.00%> (ø)
...src/met_api/services/engagement_content_service.py 95.45% <100.00%> (+7.49%) ⬆️
met-api/src/met_api/services/engagement_service.py 71.07% <100.00%> (-0.29%) ⬇️
...met_api/services/engagement_translation_service.py 88.88% <100.00%> (ø)
met-api/src/met_api/utils/enums.py 100.00% <ø> (ø)
met-web/src/apiManager/endpoints/index.ts 100.00% <ø> (ø)
... and 12 more

... and 4 files with indirect coverage changes

Copy link
Collaborator

@Baelx Baelx left a comment

Choose a reason for hiding this comment

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

Great work on this! I'm always happy to see files being deleted in the code base :)

There may be a test debugging function that needs to be removed!

@staticmethod
def _find_higest_sort_index(engagement_id):
# find the highest sort order of the engagement content
sort_index = 0
contents = EngagementContentModel.get_contents_by_engagement_id(engagement_id)
contents = EngagementContentModel.find_by_engagement_id(engagement_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for making the names less redundant here :)

@@ -5,16 +5,16 @@ import { ActionContext } from './ActionContext';
import { Skeleton } from '@mui/material';
import { getEditorStateFromRaw } from 'components/common/RichTextEditor/utils';

export const EngagementContent = () => {
const { richContent, isEngagementLoading } = useContext(ActionContext);
export const EngagementContent = ({ index = 0 }: { index?: number }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for making the old view work with this. Hopefully we can ditch this code soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope so too 😆

@@ -187,7 +192,7 @@ describe('Engagement View page tests', () => {
mockWidgetsRtkUnwrap.mockReturnValueOnce(Promise.resolve([whoIsListeningWidget]));
mockContactRtkUnwrap.mockReturnValueOnce(Promise.resolve(mockContact));
const { container } = render(<RouterProvider router={router} />);

screen.debug(undefined, 10000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be here still?

Copy link

sonarcloud bot commented Sep 3, 2024

@NatSquared NatSquared merged commit f8bd855 into main Sep 3, 2024
15 checks passed
@NatSquared NatSquared deleted the task/DESENG-676-combine-custom-and-summary-content branch September 3, 2024 18:11
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.

3 participants