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

Composition API Migration for moving towards real-time collaboration #186

Conversation

SeriousHorncat
Copy link
Collaborator

@SeriousHorncat SeriousHorncat commented Nov 8, 2024

Checklist before requesting a review

  • I have performed a self-review of my code.
  • My code follows the style guidelines enforced by static analysis tools.
  • If it is a core feature, I have added thorough tests.
  • My changes generate no new warnings.
  • New and existing unit tests pass locally with my changes.
  • Will this be part of a product update? If yes, please write one phrase about this update.

Pull Request Details

Wrike Ticket - Improving the codebase for the AnalysisView to rely on a store to setup foundation for live realtime-collaboration within Rosalution

Changes made:

  • AnalysisView uses VueJS composition API
  • AnalysisView and the analysis' state is managed using a store
  • Action menu is crafted using a VueJS composable to extract that functionality from the Analysis View
  • The hardcoded workflow was extracted to a JavaScript file that returns JSON; this is an intermediate step towards migrating it to be a setting maintained in the data layer in the future
  • Increased VueJS' version to 3.5.12 to use the TemplateRef feature released in that version
  • First steps to maintenance of the dialog code; uses the VueJS CompositionAPI and applies the TemplateRef technique to encapsulate the management of the components state programmatically
  • Improved runtime of unit tests for AnalysisView. Improved to run the view in almost half the time due to investing the time to restructure the tests to accommodate the how the code evolved.

To be done in the future:

  • Testing to be paired with the store directly instead of relying on the view indirectly covering the code
  • Improvements to the system tests to be less susceptible to timing issues for the page when rendering.

To Review:

  • Static Analysis by Reviewer

  • The changes made to the landing page of an Analysis are working as intended/rendered correctly.
    To check this run the following commands:

    # From the project root
    docker compose down
    docker system prune -a --volumes
    
    docker compose up --build
    1. Go to: https://local.rosalution.cgds/rosalution/
    2. Login as 'developer'
    3. Click into various analyses and make edits to them, save them and open in new tabs to confirm updates persist
    4. Change the status of analyses to declined, accepted, etc
    5. Import a new Rosalution analysis by clicking the 'Create New Analysis' card on Rosalution's landing page and import 'C-PAM0076.json' within the 'etc/fixtures/import'.
  • Front end unit tests run successfully on local machine

  • All GitHub Actions checks have passed.

@SeriousHorncat SeriousHorncat self-assigned this Nov 8, 2024
@SeriousHorncat SeriousHorncat marked this pull request as ready for review November 12, 2024 18:22
…ion API and applying a store to manage the view.
…s for composition api with an analysisStore and view
…ted VueJS' version to be able to use useTempalteRef
@SeriousHorncat SeriousHorncat force-pushed the composition-api-towards-websocket-integration branch from c8f24c9 to 821c13e Compare November 19, 2024 18:00
@fatimarabab fatimarabab self-requested a review November 22, 2024 16:13
@fatimarabab
Copy link
Collaborator

Everything works as expected.
Screenshot 2024-12-03 at 9 40 20 AM
Screenshot 2024-12-03 at 9 40 36 AM

All frontend unit tests pass locally! ✨
Screenshot 2024-12-03 at 9 47 09 AM

Copy link
Collaborator

@fatimarabab fatimarabab left a comment

Choose a reason for hiding this comment

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

Good to go! ✨

@SeriousHorncat SeriousHorncat merged commit 969453a into rosalution-0.8.1-patch Dec 3, 2024
8 checks passed
@SeriousHorncat SeriousHorncat deleted the composition-api-towards-websocket-integration branch December 3, 2024 21:19
SeriousHorncat added a commit that referenced this pull request Dec 11, 2024
…186)

* First steps of work in progress to migrate towards using the composition API and applying a store to manage the view.

* work in progress for revising tests for composition API and first pass for composition api with an analysisStore and view

* Fixed issues discovered during system testing with the first pass of the store migration

* first draft of testing done; cut time down in half but its still taking too long at around 300 something ms

* Finished cleaning up unit tests for the migrated toast code, and updated VueJS' version to be able to use useTempalteRef

* Renaming the RosalutionToast to ToastDialog to correspond with the other dialog components

* Secured the specific endpoints by requiring an authenticated user to access them

* Updated system test to force click in the situation the menu visibility isn't opened correctly by the test

* Fixed the 'AS' characters for the dockerfiles and fixing the accidental change as an option

* Fixed issue Rabab found in review; where the store was not switching to representing a different analysis when loading a different analysis.
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.

2 participants