-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat: add ability to edit description in settings #280
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kwajiehao
approved these changes
Sep 15, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
lamkeewei
added a commit
that referenced
this pull request
Nov 8, 2021
* develop: (25 commits) fix: package.json & package-lock.json to reduce vulnerabilities (#285) fix: package.json & package-lock.json to reduce vulnerabilities (#311) Refactor/collection refactor (#281) build(deps-dev): bump jest from 26.6.3 to 27.0.6 (#222) build(deps): bump tmpl from 1.0.4 to 1.0.5 (#304) build(deps): bump axios from 0.21.1 to 0.21.4 (#295) Fix: change git tree object format (#305) Fix: handle 409 errors when editing files (#303) hotfix: properly handle subcollection file update Fix: prevent truncating of page body (#299) Feat: add ability to edit description in settings (#280) ci: remove .ebextensions from zip ci: update backend staging eb env ci: update production eb env name (#298) ci: include .platform dir in deployment chore: specify node version chore: use EB platform hooks instead of ebextensions chore: add health check endpoint chore: use new cicd user creds fix: update ebextension to fix EB deploy ...
lamkeewei
added a commit
that referenced
this pull request
Nov 8, 2021
…token-middleware * feat/identity/database-models: (25 commits) fix: package.json & package-lock.json to reduce vulnerabilities (#285) fix: package.json & package-lock.json to reduce vulnerabilities (#311) Refactor/collection refactor (#281) build(deps-dev): bump jest from 26.6.3 to 27.0.6 (#222) build(deps): bump tmpl from 1.0.4 to 1.0.5 (#304) build(deps): bump axios from 0.21.1 to 0.21.4 (#295) Fix: change git tree object format (#305) Fix: handle 409 errors when editing files (#303) hotfix: properly handle subcollection file update Fix: prevent truncating of page body (#299) Feat: add ability to edit description in settings (#280) ci: remove .ebextensions from zip ci: update backend staging eb env ci: update production eb env name (#298) ci: include .platform dir in deployment chore: specify node version chore: use EB platform hooks instead of ebextensions chore: add health check endpoint chore: use new cicd user creds fix: update ebextension to fix EB deploy ...
lamkeewei
added a commit
that referenced
this pull request
Nov 8, 2021
…/email-login * feat/identity/site-token-middleware: (25 commits) fix: package.json & package-lock.json to reduce vulnerabilities (#285) fix: package.json & package-lock.json to reduce vulnerabilities (#311) Refactor/collection refactor (#281) build(deps-dev): bump jest from 26.6.3 to 27.0.6 (#222) build(deps): bump tmpl from 1.0.4 to 1.0.5 (#304) build(deps): bump axios from 0.21.1 to 0.21.4 (#295) Fix: change git tree object format (#305) Fix: handle 409 errors when editing files (#303) hotfix: properly handle subcollection file update Fix: prevent truncating of page body (#299) Feat: add ability to edit description in settings (#280) ci: remove .ebextensions from zip ci: update backend staging eb env ci: update production eb env name (#298) ci: include .platform dir in deployment chore: specify node version chore: use EB platform hooks instead of ebextensions chore: add health check endpoint chore: use new cicd user creds fix: update ebextension to fix EB deploy ...
alexanderleegs
pushed a commit
that referenced
this pull request
Nov 15, 2021
In PR 280, we added a description field to the Settings endpoints (see #280). This commit adds this field to the Settings V2 router as well.
alexanderleegs
added a commit
that referenced
this pull request
Nov 22, 2021
* feat: add new file services for Settings To modify our settings page, we need to introduce new file services for files which we read and write settings data from: - _config.yml - _data/footer.yml - _data/navigation.yml - index.md (homepage) The NavYmlService already exists so we don't need to create a file service for _data/navigation.yml. * feat: add new Settings router This commit adds the V2 SettingsRouter class with an implementation of the readSettingsPage route. The update route will be implemented in a subsequent commit. * refactor: extract settings files retrieval to method This commit extracts the settings file retrieval logic into a separate method on the SettingsRouter class. This is because the file retrieval logic is needed for both the read and update methods of the SettingsRouter. * refactor: update settings page This commit refactors the update route for Settings. The refactor simplifies the code structure by eliminating the use of an intermediary "update" object which tracks which files need to be updated. Using the new file services allows us to eliminate the need for formatting the content before posting to GitHub. * chore: import v2 settings router * refactor: add settings update schema * test: add sample data for tests * refactor: readSettingsPage response schema This commit refactors the readSettingsPage response to mirror the UpdateSettingsRequestSchema. This commit also refactors the readSettingsPage route function to use util functions to extract the required fields for the response. * chore: add URL attribute back to settings response * test: readSettingsPage This commit completes the test for the readSettingsPage route function. It also updates the fixtures to have the expected response from each of the page services. * chore: update settings request schema, fix logic error in updateSettingPage This commit updates the settings request schema to allow empty string values, and assigning required values correctly. It also fixes a logic error in checking whether or not to update the homepage when making a change to the title. * refactor: simplify footer update code * test: updateSettingsPage This commit completes the unit tests for the updateSettingsPage route function. * test: HomepagePageService unit tests This commit adds the unit tests for the "read" and "update" methods of the HomepagePageService. It also adds the necessary fixtures for the homepage. * test: ConfigYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the ConfigYmlService. It also adds the necessary fixtures for the config file. * test: FooterYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the FooterYmlService. It also adds the necessary fixtures for the footer file. * test: NavYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the NavYmlService. It also adds the necessary fixtures for the navigation file. * chore: add v2 settings route to auth middleware * chore: add description to Settings v2 In PR 280, we added a description field to the Settings endpoints (see #280). This commit adds this field to the Settings V2 router as well. * refactor: create new configService category This commit refactors the new Settings route to use a new SettingsService so that business logic in the routes layer is simplified - the details of HOW the Settings files are updated should not be exposed on the route layer. In doing so, we created a new configServices directory to store the SettingsService, since it does not exist as a file service. * chore: update config fixtures with description field * test: add tests for SettingsService This commit adds the necessary tests for the SettingsService, most of which were copied over and refactored from the route tests. Another change made was to use the toHaveBeenLastCalledWith method instead of the toHaveBeenCalledWith method, since it more accurately reflects the change we want to test. * test: refactor Settings route tests This commit refactors the Settings route tests so that we only mock the SettingsService class and not the underlying file services that we had used before. This commit also refactors the existing SettingsService utility methods to be static methods so that we can more easily test our code. * chore: typo * Fix: wait for expected method to fully resolve This commit modifies how we wait for a function to be completed. Previously, we would use await expect(func), but this caused some issues where not all method calls within the function were awaited properly - by changing this to await expect(func).resolves.not.toThrow(), this problem is resolved. * Fix: rebase errors * Fix: update schema to allow telegram and tiktok * fix: rebase errors * Refactor: change format of settings object * Fix: update test Co-authored-by: Alexander Lee <[email protected]>
harishv7
pushed a commit
that referenced
this pull request
Feb 17, 2023
harishv7
pushed a commit
that referenced
this pull request
Feb 17, 2023
* feat: add new file services for Settings To modify our settings page, we need to introduce new file services for files which we read and write settings data from: - _config.yml - _data/footer.yml - _data/navigation.yml - index.md (homepage) The NavYmlService already exists so we don't need to create a file service for _data/navigation.yml. * feat: add new Settings router This commit adds the V2 SettingsRouter class with an implementation of the readSettingsPage route. The update route will be implemented in a subsequent commit. * refactor: extract settings files retrieval to method This commit extracts the settings file retrieval logic into a separate method on the SettingsRouter class. This is because the file retrieval logic is needed for both the read and update methods of the SettingsRouter. * refactor: update settings page This commit refactors the update route for Settings. The refactor simplifies the code structure by eliminating the use of an intermediary "update" object which tracks which files need to be updated. Using the new file services allows us to eliminate the need for formatting the content before posting to GitHub. * chore: import v2 settings router * refactor: add settings update schema * test: add sample data for tests * refactor: readSettingsPage response schema This commit refactors the readSettingsPage response to mirror the UpdateSettingsRequestSchema. This commit also refactors the readSettingsPage route function to use util functions to extract the required fields for the response. * chore: add URL attribute back to settings response * test: readSettingsPage This commit completes the test for the readSettingsPage route function. It also updates the fixtures to have the expected response from each of the page services. * chore: update settings request schema, fix logic error in updateSettingPage This commit updates the settings request schema to allow empty string values, and assigning required values correctly. It also fixes a logic error in checking whether or not to update the homepage when making a change to the title. * refactor: simplify footer update code * test: updateSettingsPage This commit completes the unit tests for the updateSettingsPage route function. * test: HomepagePageService unit tests This commit adds the unit tests for the "read" and "update" methods of the HomepagePageService. It also adds the necessary fixtures for the homepage. * test: ConfigYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the ConfigYmlService. It also adds the necessary fixtures for the config file. * test: FooterYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the FooterYmlService. It also adds the necessary fixtures for the footer file. * test: NavYmlService unit tests This commit adds the unit tests for the "read" and "update" methods of the NavYmlService. It also adds the necessary fixtures for the navigation file. * chore: add v2 settings route to auth middleware * chore: add description to Settings v2 In PR 280, we added a description field to the Settings endpoints (see #280). This commit adds this field to the Settings V2 router as well. * refactor: create new configService category This commit refactors the new Settings route to use a new SettingsService so that business logic in the routes layer is simplified - the details of HOW the Settings files are updated should not be exposed on the route layer. In doing so, we created a new configServices directory to store the SettingsService, since it does not exist as a file service. * chore: update config fixtures with description field * test: add tests for SettingsService This commit adds the necessary tests for the SettingsService, most of which were copied over and refactored from the route tests. Another change made was to use the toHaveBeenLastCalledWith method instead of the toHaveBeenCalledWith method, since it more accurately reflects the change we want to test. * test: refactor Settings route tests This commit refactors the Settings route tests so that we only mock the SettingsService class and not the underlying file services that we had used before. This commit also refactors the existing SettingsService utility methods to be static methods so that we can more easily test our code. * chore: typo * Fix: wait for expected method to fully resolve This commit modifies how we wait for a function to be completed. Previously, we would use await expect(func), but this caused some issues where not all method calls within the function were awaited properly - by changing this to await expect(func).resolves.not.toThrow(), this problem is resolved. * Fix: rebase errors * Fix: update schema to allow telegram and tiktok * fix: rebase errors * Refactor: change format of settings object * Fix: update test Co-authored-by: Alexander Lee <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds functionality to allow the editing of the description field in both the
config.yml
file as well as the homepage. To be reviewed in conjunction with PR #613 on the isomercms frontend repo.