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

Vithu Clean Architecture Implementation for SessionUseCase and MessageUseCase #31

Merged
merged 8 commits into from
Nov 27, 2023

Conversation

vithushon
Copy link
Contributor

@vithushon vithushon commented Nov 26, 2023

Description

Implemented Clean Architecture for the SessionUseCase(s) and MessageUseCase(s) as part of secondment to Callhub Connect. Removal of certain files such as MessageWebSocketAccess was a consequence of restructuring the MessageUseCase. Will need reviewers to check if it meets requirements.

Fixes # N/A

Type of change

  • Implementing Clean Architecture (non-breaking change)

How Has This Been Tested?

Did not run any tests on this code. I did run the frontend application and ensured that functionality remained the same after implementation. The code should function exactly the same way it did prior to CA implementation.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • My changes generate no new warnings

…ession and JoinSession. May be worth splitting into two separate use cases - should be decided by CallHub.
…ages in the data_access folder to organize the files better.
Copy link
Collaborator

@sarinali sarinali 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!! I don't know how I would have done this LOL

Comments:

  • Just some code styling, maybe make the new line close squiggly bracket consistent among all files and new lines ,
    otherwise looks good to me!

Copy link
Collaborator

@sarinali sarinali left a comment

Choose a reason for hiding this comment

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

Can you resolve merge conflict so the checks can run?

@vithushon
Copy link
Contributor Author

Yup, will take a look at it tonight and review the code styling + try to resolve the conflict. Will keep you posted!

Copy link
Contributor

@zjayee zjayee left a comment

Choose a reason for hiding this comment

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

Everything works for me locally! Thank you so much for doing this for us!

@vithushon
Copy link
Contributor Author

Great! I think I resolved the merge conflicts. Though, you may need to remove the class ManageWebSocketAccess at a later point as it's just empty and I put it in the driver folder to help resolve the conflict. Should be fine to remove it and keep functionality as required.

@vithushon
Copy link
Contributor Author

vithushon commented Nov 27, 2023

Just saw the failed check - It might have to do with the subdirectories in the data_access folder. Not sure how to change the location in which the check looks for the appropriate files.

Edit: removed the subdirectories and now the check seems to pass. Should be okay now I think - may be worth to make sure the program still runs appropriately on your end.

@zjayee zjayee merged commit 3d2dbc1 into master Nov 27, 2023
1 check passed
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