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

Merge user and question microservices #33

Merged

Conversation

iynixil
Copy link

@iynixil iynixil commented Oct 1, 2024

note: this merging of services is built upon user-service branch

fetch this PR changes onto local repo: (since its not pushed into the merge-user-question branch yet)
git fetch upstream pull/33/head:<branch name>
git checkout <branch name>
this creates a new branch in your local repo with the <branch name> to view this PR's changes

Changes:

change file structure of backend/:

  • every microservice in its own directory
  • every microservice has controller and routes folders
  • common files used across services are left in backend/ e.g firebase.js
  • changed command to run backend server: in backend/package.json, npm start was node index.js, now npm start is nodemon index.js
    • this allows changes to be reflected instantly when file is changed without restarting backend server

change file structure of frontend/:

  • renamed frontend folder from my-app/ -> frontend/
  • put all pages under frontend/src/pages/, pages separated by microservice
  • each microservice page directory has styles and utils folder
    • styles for .css files, utils for storing utility functions (can delete utils at the end of project before submission if not used)
  • put common components used across all pages under frontend/src/components
    • components has styles folder for .css files

general:

  • added a comment at the top of every file, Author(s): to include names of people who created most of the functionality used in the file. makes it easier to write individual contribution later on
  • the app looks different because the .css files are conflicting with each other

if there are issues with functionality/etc, can let me know

tsulim and others added 30 commits September 11, 2024 23:34
…anch_xl

Make changes according to coding standard
New action column for editting that data's row
add new button to delete for each row
backend delete based on id
tsulim and others added 24 commits September 29, 2024 20:11
…ranch-tsu

Display selected question's details
…ranch_xl

Add scroll back to top for edit button
…ranch_xl

Handle duplicate question title
…ranch-tsu

Extract question routes to reduce clutter
@iynixil iynixil closed this Oct 1, 2024
@iynixil iynixil reopened this Oct 1, 2024
Copy link

@xuelinglow xuelinglow left a comment

Choose a reason for hiding this comment

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

tested and lgtm

Copy link

@whitesnowx whitesnowx 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 me and works fine on correct use

I think this could fall under n2h securities concern, but not sure whether you want to fix this in this milestone:

image
Injection using /" crashed the backend

@iynixil iynixil merged commit 18cb036 into CS3219-AY2425S1:merged-user-question Oct 2, 2024
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.

4 participants