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

[Feature Branch] Analysis Discussion Section - Phase 1 #161

Merged
merged 19 commits into from
Feb 9, 2024

Conversation

JmScherer
Copy link
Collaborator

@JmScherer JmScherer commented Feb 6, 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

https://www.wrike.com/open.htm?id=1206929278
https://www.wrike.com/open.htm?id=1206933589

Changes made:

Analysis Discussion Section

Adds a new discussion to the Analysis. This section allows for users to post plaintext to be seen in an analysis. This allows for users to share thoughts/opinions/outlook on an analysis. They also have the ability to edit and delete posts that they made within the discussion section.

  • Adds a whole new section called Discussion to every analysis in Rosalution.
  • Modifies the RosalutionHeader.vue to include the new section
  • Adds frontend components to manage the discussion section in vue
    • DiscussionSection.vue is the parent container the contains a collection of discussion posts
    • DiscussionPost.vue is the child component which displays the contents of each individual post
  • New analysis.js model functions to handle the interaction between the front and back end
    • These functions handle Add, Edit, and Delete discussion posts
  • This section includes a unique interface to add a discussion post and have it show up in the section
    • New Discussion button opens a field that allows free text to be entered into Rosalution
  • Adds a context menu for a user to select an action for a discussion post
  • Ensures that the user can only perform these actions on discussions they posted
    • The user authorization object was changed to always include the client_id to match which posts are made by the user
  • Adds backend routes that handle how discussion posts are interacted with:
    • @router.get("/{analysis_name}/discussions")
    • @router.post("/{analysis_name}/discussions")
    • @router.put("/{analysis_name}/discussions/{discussion_post_id}")
    • @router.delete("/{analysis_name}/discussions/{discussion_post_id}")
  • Adds new collection functions inside analysis_collection.py to handle mongo interactions
  • Adds relevant testing including system tests for ensuring this functionality works as intended

To Review:

  • Static Analysis by Reviewer
  • Add, Edit, and Delete a discussion post
    To check this run the following commands:
    # From the root of Rosalution
    docker compose down
    docker system prune -a --volumes
    
    docker compose up --build
    • Go to: https://local.rosalution.cgds/rosalution/
    • Login with researcher
    • Click on CPAM0002
    • Scroll down to the Discussion section
    • Click on New Discussion button
    • Enter some text in the field and click Publish
    • Find your new post in the Discussion Section and click the ... and then click Edit
    • Enter new text and click Save
    • Find the same post and click the ... and then click Delete
    • Click Delete in the confirmation menu
    • Ensure that the post was removed from the Discussion section by refreshing the page
  • Bonus: System Tests cover other cases such as canceling the actions, but give it a try to ensure it works
  • All Github Actions checks have passed.

Discussion Section as researcher

Screenshot 2024-02-06 at 10 15 28 AM

Adding a new discussion post

Screenshot 2024-02-06 at 10 19 26 AM

Added Discussion Post

Screenshot 2024-02-06 at 10 20 05 AM

Edit Discussion Post through context menu

Screenshot 2024-02-06 at 10 20 37 AM

Editing a Discussion Post In-place

Screenshot 2024-02-06 at 10 22 36 AM

Edited post in the discussion section

Screenshot 2024-02-06 at 10 22 53 AM

Deleting a Discussion Post through context menu

Screenshot 2024-02-06 at 10 23 46 AM

Discussion Post Delete Confirmation

Screenshot 2024-02-06 at 10 24 35 AM

Discussion Post Deleted from the Section

Screenshot 2024-02-06 at 10 24 51 AM

JmScherer and others added 16 commits November 27, 2023 09:16
…alysis view (#139)

* Updating the paper to include the country for each affiliation which is required for publication.

* Updated the system tests workflow to use electron (#140)

* Added a DiscussionsSection.vue in AnalysisView components, updated the analysis model to return an injected discussion object, and updating the styling of the buttons to match figma more closely

* The discussion collapse works

* Looks ready for a pull request

* Tests should be working now

* Left out the node test

* Updating CSS in a few places

* Updating the DiscussionSection values

---------

Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
…osts within them. Removed the temp fixture used in the frontend (#141)
* New Discussions post button, text field, and save/publish buttons

* Creating temporary discussion API mock api endpoints to test integrating them into the frontend

* Displaying posts from an analysis

* Hooked up the discussion post to the backend and return a mock discussions post. Added styling to the discussion posts

* Updated CSS to alternate discussion post colors

* Added more unit test coverage on the frontend and linted

* Added system tests and data-test attributes to go along with it

---------

Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
…he header line divider to enter an opinion (#151)

* New Discussions post button, text field, and save/publish buttons

* Displaying posts from an analysis

* Added more unit test coverage on the frontend and linted

* Toggles New Discussion Field

* added system and unit tests for the discussion section

* forgot to add the system test

* Minor CSS fixes
…nd save it in Mongo (#153)

* Added a new collection to update a new post along with a test and linting

* Fixing python unit test

* Integration test and linting

* Updated the analysis_collection add_discussion_post function to use pymongo find_one_and_update function to simplify how posts are added to discussions in analyses

* linting
…cussions keys are added to each analysis object in the analyses collection (#154)
…iscussions field. Updated the analyses fixture to include these fields as well (#155)
* added actions to the discussion post context menus

* Changed the backend user object to send back the clientId in the basic user object, this is used to check if the user made a post and present a context menu

* new ContextMenu.vue duplicates the DropDownMenu.vue functionality and turns it from hover to click. This is not quite right

* lots of changes: swapped dropdown and contextmenu icons, contextmenu will now open on click and close when unfocused or an action is taken on the menu. ContextMenu styling changes

* Fixed frontend test

* Lots of fixes, trying to figure out how to test this

* Frontend linting

* Removed a .only in front end test
… a post from the discussion section (#157)

* Added a delete route and delete post analysis collection function

* Added a new route for editing a post and matching collection function, also added error checking for different situations in modifying discussion posts in an analysis

* Change responsibilities of routes and put helper function to find discussion post in analysis model

* proper error handling for discussion posts in progress

* Removed the discussion fixtures and properly gets the discussion posts from the analysis

* Finished integration and unit tests for updating and deleting a discussion post

* Formatting/linting

* Fixed integration test

* Added unit and integration tests for analysis model and routers

* Update backend/src/routers/analysis_discussion_router.py

Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: James Scherer <james.m.scherer@gmail.com>

* Update backend/tests/integration/test_analysis_routers.py

Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: James Scherer <james.m.scherer@gmail.com>

* linting

---------

Signed-off-by: James Scherer <james.m.scherer@gmail.com>
Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
…158)

* Frontend discussion post deletion working, complete with notification dialog

* Adding a system test for successfully deleting a new discussion post

* fixing frontend unit tests and adding an extra system test

* Linting

* Changed the delete action in the context menu to have an emit key and chain the emits down the section to be called in the view to delete a post

* Linting and changing var names

* Lots of frontend unit tests, but not yet finished

* Added more tests and linting

* Fixed issue with contextId being renamed

* Removed console log
)

* Frontend discussion post deletion working, complete with notification dialog

* Adding a system test for successfully deleting a new discussion post

* fixing frontend unit tests and adding an extra system test

* Linting

* Changed the delete action in the context menu to have an emit key and chain the emits down the section to be called in the view to delete a post

* Linting and changing var names

* Lots of frontend unit tests, but not yet finished

* Added more tests and linting

* Fixed issue with contextId being renamed

* Removed console log

* Edit post emit chain working and backend is saving properly, it's just the discussion content is the same as what's posted

* Now editing a post works, it edits the post in place

* More styling for the post editing

* Frontend linting

* Frontend tests and linting

* Added system tests for post editing
@JmScherer JmScherer added the enhancement New feature or request label Feb 6, 2024
@JmScherer JmScherer self-assigned this Feb 6, 2024
Copy link
Collaborator

@SeriousHorncat SeriousHorncat left a comment

Choose a reason for hiding this comment

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

Doing the review in multiple steps. Just did a step through validation of the feature.

  1. Leaving a post as a 'researcher' user
    image
  2. Editing a post as a 'researcher' user
    image
  3. A developer responding to a researchers
    image
  4. Researcher deleting their post due to not having time for this deep of a discussion
    image

Copy link
Collaborator

@SeriousHorncat SeriousHorncat left a comment

Choose a reason for hiding this comment

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

I just did a first pass; Checking a few things and will a more thorough static analysis tomorrow. So excited for this to make it into main soon!

.github/workflows/system-tests.yml Outdated Show resolved Hide resolved
backend/src/routers/analysis_router.py Outdated Show resolved Hide resolved
@fatimarabab
Copy link
Collaborator

fatimarabab commented Feb 7, 2024

The Discussion Section and Post components are working as expected. Everything looks great! Going to try running system tests next, before approving.

Screenshot 2024-02-07 at 5 21 43 PM
Screenshot 2024-02-07 at 5 22 12 PM

@fatimarabab
Copy link
Collaborator

One of the system tests is failing for me - rosalution.analysis.cy.js
Screenshot 2024-02-08 at 10 44 31 AM
Screenshot 2024-02-08 at 10 45 26 AM

@fatimarabab
Copy link
Collaborator

One of the system tests is failing for me - rosalution.analysis.cy.js Screenshot 2024-02-08 at 10 44 31 AM Screenshot 2024-02-08 at 10 45 26 AM

This needs to be refactored in a separate branch as it is unrelated to this PR. This Wrike Ticket was created for this and added to Rosalution>Backlog>Technical Debt - https://www.wrike.com/open.htm?id=1299739145

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.

Looks good to go!

JmScherer and others added 3 commits February 8, 2024 14:18
Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: James Scherer <james.m.scherer@gmail.com>
Co-authored-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: James Scherer <james.m.scherer@gmail.com>
@JmScherer JmScherer merged commit 82cda52 into main Feb 9, 2024
8 checks passed
@JmScherer JmScherer deleted the feature-collab-analysis-discussion branch February 9, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants