-
Notifications
You must be signed in to change notification settings - Fork 2
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/buttons #585
Merged
Merged
Feat/buttons #585
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
14a1341
to
7dd770d
Compare
kishore03109
suggested changes
Dec 8, 2022
kishore03109
approved these changes
Dec 12, 2022
03610da
to
1b0ae89
Compare
1b0ae89
to
3db9533
Compare
eb89781
to
d59f2d6
Compare
…d but it's opaque atm
* fix(sites.spec): update imports * test(sitesservice): update types * test(reviewrequestservice.spec): update spce * test(collaboratorsservice): fixed tests * chore(databaseerror): add new error * fix(sitesservice): fixed regression in code * test(sitesservice.spec): fixed tests * chore(sitesservice): remove extra console log * chore(sites/user spec): add sync to prevent db clash * chore(databaseerror): removed extra properties * chore(constants): add new constant for homepage name * fix(pageservice): fixed bug allowing empty collections * chore(constants): add new constant for contact-us * refactor(pages): rename property * test(pageservice.spec): add tests * chore(constants): rename homepage constants for consistency * chore(pageservice): remove redundant code * chore(pageservice.spec,): remove extra async * chore(pageservice.spec): standardise imports \ * refactor(pageservice): update parsecolletionpage * test(pageservice.spec): update tests for new error mesasge * fix(pageservice.spec): removed extra newline * test(integration): fixed reviews integration tests * test(review): updated spec for new api * tests(notifs + page): fixed failing tests * test(review): fix for dev changes
alexanderleegs
pushed a commit
that referenced
this pull request
Apr 11, 2023
* feat(reviewrequestservice): add initial impl to copute url * chore(errors): add many errors * feat(types): add types * refactor(deployment): chaneg string type to permalink * feat(pageservice): add new pageservice abstration for raw js wrappers * feat(json): add new safeJsonParse util * refactor(reviewrequestservice): refactor to use new page service * refactor(sitesservice): refactor to use new page service * refactor(review): refactor for neverthrow api * chore(server): init page service * chore(pageservice): add comment * fix(reviewrequestservice): add in await * fix(collaboratorsservice): add in unwrap * chore(review.ts): removed vapt fixes * chore(services/types): renamed variables and methods for clarity * chore(errors): removed unneeded properties * fix(server): u9pdate after rebase * fix(review): update after rebase * fix(rebase fixes): add cast cos the `permalink` property is guaranteed but it's opaque atm * chore(server): add code for init of pageservice * chore(rr service): specify return types * fix(notif handler): update to fit new api * test(app): updated specs to fit with new API (#587) * fix(sites.spec): update imports * test(sitesservice): update types * test(reviewrequestservice.spec): update spce * test(collaboratorsservice): fixed tests * chore(databaseerror): add new error * fix(sitesservice): fixed regression in code * test(sitesservice.spec): fixed tests * chore(sitesservice): remove extra console log * chore(sites/user spec): add sync to prevent db clash * chore(databaseerror): removed extra properties * chore(constants): add new constant for homepage name * fix(pageservice): fixed bug allowing empty collections * chore(constants): add new constant for contact-us * refactor(pages): rename property * test(pageservice.spec): add tests * chore(constants): rename homepage constants for consistency * chore(pageservice): remove redundant code * chore(pageservice.spec,): remove extra async * chore(pageservice.spec): standardise imports \ * refactor(pageservice): update parsecolletionpage * test(pageservice.spec): update tests for new error mesasge * fix(pageservice.spec): removed extra newline * test(integration): fixed reviews integration tests * test(review): updated spec for new api * tests(notifs + page): fixed failing tests * test(review): fix for dev changes
Closed
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.
tests coming in a follow up PR yeet
Problem
Previously, there was no api providing a translation from filename to staging url. This PR adds this service in.
Moreover, as part of providing this service, other tangential services have also been mildly reworked to accept a result-esque api.
Though this is rudimentary at present, we should refine this more in the future so that our APIs are safe.
also short writeup here for reasoning behind how service operates
Solution
PageService
that parses filenames into their respective typesfile_url
. For files/ext links, this should be fast b/c the actual file in the resource folder contains only metadata.ResultAsync
based APIs rather thanPromise
based.Site/ReviewRequestService
and only for some methods. The way this is done now is very crude and we should aim to return better errors rather thanNotFoundError
. This is just done as a base so that we can change the error type to be a more specific one in the futurewithError
method inPageService
should be extracted out in the future and used for interfacing with both.js
files that call github APIs (either directly or indirectly throughGithubService
) and.ts
files that call some external endpointsNext steps
PageService
should also aim to parse the page type (or defer it to some otherFileService
) as we have config files etc that are not tangentially related to what the user wants to see