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

Ctskf 832 - CCCD - Allow for multiple documents on a message #8064

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

VinceChiuMOJ
Copy link
Contributor

@VinceChiuMOJ VinceChiuMOJ commented Jan 16, 2025

Ticket:

CCCD - Allow for multiple documents on a message

https://dsdmoj.atlassian.net/browse/CTSKF-832


241006 updates:

Changed the attachment variable name to attachments (plural for multiple). New attachments (one) can be added and download afterwards. The old attachment did not work anymore. Fixed by updating the name column in active_storage_attachments.

241025 updates:

Tried to modified rspec tests to pass.
Accidentally fixed the issue for remove file link not showing for attachments.

250113 updates:

Checked out a new branch to refine the changes did before on top of CTSKF-1002.

  • spec/models/message_spec.rb:
    No idea why there is no change in blob count, will further investigate

250122 updates:

Passed all the tests but there are some uncertainties.

  • spec/models/message_spec.rb:
    Commented out the tests for blob count change. This test may not be applicable when we move on to CTSKF-833 where we create the attachments as documents. Thus the blob will still exist after we remove the message.

  • features/step_definitions/messaging_steps.rb:
    Not sure if it is OK not using those hooks.

@VinceChiuMOJ VinceChiuMOJ requested review from a team as code owners January 16, 2025 14:47
Copy link
Contributor

@mpw5 mpw5 left a comment

Choose a reason for hiding this comment

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

I've left some comments and questions - happy to discuss if you like 🙂

features/step_definitions/messaging_steps.rb Outdated Show resolved Hide resolved
features/step_definitions/messaging_steps.rb Outdated Show resolved Hide resolved
spec/models/message_spec.rb Outdated Show resolved Hide resolved
app/models/message.rb Outdated Show resolved Hide resolved
@mpw5 mpw5 self-requested a review January 28, 2025 16:35
Copy link
Contributor

@mpw5 mpw5 left a comment

Choose a reason for hiding this comment

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

👍

@VinceChiuMOJ VinceChiuMOJ force-pushed the CTSKF-832_250116 branch 2 times, most recently from 536f5d2 to e75e45a Compare February 4, 2025 18:14
refine the original changes on top of CTSKF-1002
Copy link

sonarqubecloud bot commented Feb 5, 2025

@VinceChiuMOJ VinceChiuMOJ changed the title Ctskf 832 250116 Ctskf 832 - CCCD - Allow for multiple documents on a message Feb 6, 2025
@VinceChiuMOJ VinceChiuMOJ merged commit ed006ad into master Feb 6, 2025
10 of 11 checks passed
@VinceChiuMOJ VinceChiuMOJ deleted the CTSKF-832_250116 branch February 6, 2025 09:39
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