Skip to content

Multiapplication Support #359

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

Merged
merged 12 commits into from
Sep 4, 2023
Merged

Multiapplication Support #359

merged 12 commits into from
Sep 4, 2023

Conversation

brianzhang2101
Copy link
Contributor

@brianzhang2101 brianzhang2101 commented May 30, 2023

Generalising the CMS to enable other teams to utilise it. Improving user experience & security issues with sharing files in one large repository.

  • New frontend table maps from URL, Name to an isolated root in the filesystem table which contains all files/directories branches
    • Helper function added to set these up
  • Schema now supports the concept of groups and permissions to read, write and delete a collection of applications or frontends (helper functions not in place yet)
  • FrontendID now uses UUID
  • Updated and reorganised Postgres testing data to better emulate data flow better

Update (rest of the specifics above are same):

  • Small refactor on schema to now have an ID as primary key on frontend table and a separate root column representing the root inside the filesystem table
  • Setup functions will handle UUID generation (no more code-side generation, which was inconsistent and confusing to read in tests)

Copy link
Member

@Varun-Sethu Varun-Sethu 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, some minor comments though and also:

  • Since I think the original goal of the ticket kinda changed its probs worth adding a full list of changes in the PR description
  • I think you may need to regenerate mocks as you did end up refactoring the repository layer quite a bit.
    • You will need to update
      //go:generate mockgen -source=models.go -destination=mocks/models_mock.go -package=mocks
      to be //go:generate mockgen -source=repository_interfaces.go -destination=mocks/repositories_mock.go -package=mocks
    • And then run go generate ./... at the top level in the backend directory

@Varun-Sethu Varun-Sethu self-requested a review August 27, 2023 03:38
@Varun-Sethu Varun-Sethu removed the request for review from hdphuong August 27, 2023 03:51
@ShunyaoLiang
Copy link
Contributor

Thanks for the meeting Brian. Looks great to me, just want there to be unit tests for multiple frontend repositories at once.

@brianzhang2101
Copy link
Contributor Author

brianzhang2101 commented Sep 3, 2023

Luckily this test did actually spot a bug where GetIDWithPath(path string) forgets to query specifically within the FE's filetree, thanks @ShunyaoLiang

Copy link
Contributor

@ShunyaoLiang ShunyaoLiang left a comment

Choose a reason for hiding this comment

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

That new test looks good to me. Great work Brian!

@brianzhang2101 brianzhang2101 merged commit 5545f74 into main Sep 4, 2023
@brianzhang2101 brianzhang2101 deleted the multiapplication_support branch September 4, 2023 02:22
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