Update all dependencies to next major version and fix issues caused by update#26
Open
Computroniks wants to merge 14 commits into
Open
Update all dependencies to next major version and fix issues caused by update#26Computroniks wants to merge 14 commits into
Computroniks wants to merge 14 commits into
Conversation
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Editor config should not be included in the repo Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
This method incorrectly caught all errors from the Wildduck API as internal server errors hence failed when adding a new domain Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
The user ID is passed as a string, not an ObjectId so toHexString fails here Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
For some unknown reason, the data for a job when it is run no longer contains the user _id as an ObjectId, even though when the job was added to the queue, it was an ObjectId. Instead it emerges as a string. To fix, just convert back to ObjectId before we do anything else with the job. Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
For some reason, if we try to run .save() after removing the last domain in a users list, the empty array is not saved and the last domain is not deleted. Domains can be deleted if there are more than one, however the very last domain could not. A similar issue was seen when adding an alias, hence that has also been changed to .update() The return types of the two methods have been changed, however they are only used in one place each and the return values are not used so there is no issue here. Signed-off-by: Matthew Nickson <mnickson@sidingsmedia.com>
Author
|
@louis-lau What would your thoughts be on potentially getting this merged? Currently I have this running on a testing system and it seems to be running ok from the basic admin I have done |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 updates all the dependencies to their most recent major version and fixes all the issues encountered by that update.
In terms of what I have actually changed:
npm installsuccessfully with this still installed as it kept asking for an API key and causing the command to fail).vscodedirectory (Probably shouldn't keep editor configs in the repo)@nestjs/axiospackage as@nestjs/commondoesn't includeHttpModuleorHttpServicenowtruefrom the second argument tonew Logger()calls because this is no longer valid`ObjectIDtoObjectIdnestjs-consoleno longer includes a spinner and I didn't want to add an extra dependency here just for a spinnerawaitkeyword in api-keys.service when creating an API key. Without thisawaitthe API key is generated but doesn't actually get saved in the database so is unusable.findOnewithfindOneByclassToClasstoinstanceToInstanceas a result of a name change in the package supplying thesejsonParsein the config service to handle new@TransformparametersresolveDkimIdmethod. The catch block would be run on a404response, meaning that the expected404would result in DuckyAPI thinking there was an internal server error and failing to add the domain, among other issues.main.tsfixed how we get the config service to match new nestjs versiondelete-for-domain.processorfor some reason the data for the job ends up with the user_idbeing a string, even though when the job was added to the queue it was anObjectIdso just make sure that it is definitely anObjectId. More detail in 4f61406.save()to.update()forpushAliasandpullDomain. More details about that in 919a223I have tested all the functionality from the web UI and it all appears to function correctly, however as this is a fairly large change, if other people could also double check, it would be greatly appreciated.