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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes for project, project-permissions services hook refactor #8989

Merged
merged 20 commits into from Oct 16, 2023

Conversation

MoizAdnan
Copy link
Member

@MoizAdnan MoizAdnan commented Oct 5, 2023

Summary

馃 Generated by Copilot at 21bdbe9

This pull request refactors and updates several files and modules related to the project, project permission, scene, and route services, as well as some scripts and tests. The main purpose is to improve the compatibility, security, consistency, and readability of the code, and to fix some errors and issues. The changes include using public methods instead of internal methods of the projectPath and projectPermissionPath services, extending the KnexService class instead of the KnexAdapter class, adding and modifying hook functions and validations, and adjusting some method signatures and parameters.

References

#8871

Explanation

馃 Generated by Copilot at 21bdbe9

  • Changed the update method calls of the KnexAdapter and projectPath services to pass an empty string as the first argument, instead of the data object, to avoid errors and match the expected format (link, link, link, link, link, link)
  • Changed the ProjectPermissionService and ProjectService classes to extend the KnexService class instead of the KnexAdapter class, to leverage its features and be consistent with other services (link, link)
  • Removed the app property and the constructor that assigned it from the ProjectPermissionService and ProjectService classes, as they were unnecessary and inherited from the KnexService class (link, link)
  • Changed the service method calls that used the internal _ methods to use the public methods instead, to ensure that the hooks and validations are applied (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Added the ensureInviteCode, checkExistingPermissions, checkUser, and checkPermissionStatus hook functions to the project-permission hooks file, to validate and adjust the project permission data and queries (link, link, link, link)
  • Added the createAndUploadProject, updateProjectFiles, removeProjectFiles, addDataToProjectResult, checkProjectExists, checkProjectWriteAccess, checkProjectGithubUrl, and checkProjectAutoUpdate hook functions to the project hooks file, to perform various tasks related to creating, updating, deleting, and querying projects (link)
  • Added the new hook functions to the before and after hooks of the project and project-permission services, according to the logic and order of the operations ( link)
  • Made the needsRebuild and reset fields of the ProjectBuildUpdateItemSchema optional, to allow more flexibility and compatibility (link)
  • Imported the INVITE_CODE_REGEX and USER_ID_REGEX constants from the IdConstants module, and the GITHUB_URL_REGEX constant from the GitHubConstants module, to validate the invite code, user id, and GitHub URL fields in the hooks (link, link)
  • Imported the projectPath constant from the project.schema module, and the fs and path modules, to access the project service and manipulate the file system and paths in the hooks (link, link)
  • Imported several constants, types, and modules related to the avatar, location, route, static resource, identity provider, and github repo access schemas and services, to remove the associated data when deleting a project in the hooks (link)
  • Removed the placeholder parameter from the update method of the ProjectService class, and reordered the other parameters, to match the signature of the update method of the KnexService class (link)
  • Removed the projectPath import from the ProjectPermissionService class, as it was not used (link)
  • Removed the semicolon at the end of the installAllProjects function call in the install-projects script file, as it was unnecessary and inconsistent (link)
  • Removed the empty line at the beginning of the install-projects script file, as it was unnecessary and inconsistent (link)
  • Reordered the imports in the install-projects script file alphabetically, as it was a code style preference (link)
  • Wrapped the results of some patch and find method calls in parentheses and cast them as the expected types, to avoid TypeScript errors and make the code work (link, link, link)

馃 Generated by Copilot at 21bdbe9

The project service got a makeover
With KnexService and hooks to take over
The private methods were shunned
The public ones were fun
And the scripts and tests were much safer

QA Steps

List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.

Checklist

  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • ensure all checks pass
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewers

@MoizAdnan MoizAdnan marked this pull request as ready for review October 10, 2023 12:08
Made checking for project permissions only apply to external calls
@hanzlamateen hanzlamateen self-requested a review October 15, 2023 10:38
@barankyle barankyle added this pull request to the merge queue Oct 16, 2023
Merged via the queue into dev with commit 6ad6a6f Oct 16, 2023
13 checks passed
@barankyle barankyle deleted the project-hooks branch October 16, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants