Telodendria/Telodendria
Telodendria/Telodendria
3
7
Fork 10
Code Issues 8 Pull requests 1 Projects 3 Releases 4 Wiki Activity Actions

Implement #27 #36

Merged
jordan merged 37 commits from lda/telodendria:implement-27 into master 2023-09-25 13:39:24 +00:00
Conversation 1 Commits 37 Files changed 7 +176 -5
lda commented 2023-09-17 18:15:24 +00:00
Contributor
Copy link

Implements #27.

Please review the developer certificate of origin:

  1. The contribution was created in whole or in part by me, and I have
    the right to submit it under the open source licenses of the
    Telodendria project; or
  2. The contribution is based upon a previous work that, to the best of
    my knowledge, is covered under an appropriate open source license and
    I have the right under that license to submit that work with
    modifications, whether created in whole or in part by me, under the
    Telodendria project license; or
  3. The contribution was provided directly to me by some other person
    who certified (1), (2), or (3), and I have not modified it.
  4. I understand and agree that this project and the contribution are
    made public and that a record of the contribution—including all
    personal information I submit with it—is maintained indefinitely
    and may be redistributed consistent with this project or the open
    source licenses involved.
  • I have read the Telodendria Project development certificate of
    origin, and I certify that I have permission to submit this patch
    under the conditions specified in it.
Implements #27. --- Please review the developer certificate of origin: 1. The contribution was created in whole or in part by me, and I have the right to submit it under the open source licenses of the Telodendria project; or 1. The contribution is based upon a previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the Telodendria project license; or 1. The contribution was provided directly to me by some other person who certified (1), (2), or (3), and I have not modified it. 1. I understand and agree that this project and the contribution are made public and that a record of the contribution—including all personal information I submit with it—is maintained indefinitely and may be redistributed consistent with this project or the open source licenses involved. - [x] I have read the Telodendria Project development certificate of origin, and I certify that I have permission to submit this patch under the conditions specified in it.
lda added 30 commits 2023-09-17 18:15:26 +00:00 
Fixes #33.

Co-authored-by: LoaD Accumulator <lda@freetards.xyz>
Reviewed-on: Telodendria/telodendria#35
Co-authored-by: LoaD Accumulator <lda@noreply.git.telodendria.io>
Co-committed-by: LoaD Accumulator <lda@noreply.git.telodendria.io>
lda added 1 commit 2023-09-17 18:18:21 +00:00
lda added 1 commit 2023-09-17 18:20:59 +00:00
lda added 1 commit 2023-09-17 18:48:09 +00:00
lda added 1 commit 2023-09-17 18:50:34 +00:00
lda added 1 commit 2023-09-20 06:11:54 +00:00
jordan reviewed 2023-09-20 19:51:19 +00:00
src/Routes/RouteAdminDeactivate.c
@ -0,0 +116,4 @@
}
finish:
if (user)
jordan commented 2023-09-20 19:51:19 +00:00
Owner
Copy link

This shouldn't be necessary. It should suffice to just UserUnlock() here without first checking for NULL, because UserUnlock() should have a NULL check in it already. I am going off of memory on that one, but if I'm wrong, UserUnlock() should have this check instead.

This shouldn't be necessary. It should suffice to just `UserUnlock()` here without first checking for `NULL`, because `UserUnlock()` should have a `NULL` check in it already. I am going off of memory on that one, but if I'm wrong, `UserUnlock()` should have this check instead.
lda marked this conversation as resolved
jordan reviewed 2023-09-20 19:51:36 +00:00
src/Routes/RouteAdminDeactivate.c
@ -0,0 +120,4 @@
{
UserUnlock(user);
}
if (removed)
jordan commented 2023-09-20 19:51:36 +00:00
Owner
Copy link

Same as above.

Same as above.
lda marked this conversation as resolved
jordan reviewed 2023-09-20 19:53:11 +00:00
src/Routes/RouteAdminDeactivate.c
@ -0,0 +50,4 @@
if ((method != HTTP_DELETE) && (method != HTTP_PUT))
{
char * msg = "Route only supports DELETE and PUT as for now.";
jordan commented 2023-09-20 19:53:11 +00:00
Owner
Copy link

Is there a reason for declaring a variable here instead of just passing the string literal into MatrixErrorCreate()?

Is there a reason for declaring a variable here instead of just passing the string literal into `MatrixErrorCreate()`?
lda commented 2023-09-21 06:00:39 +00:00
Author
Contributor
Copy link

Honestly, it's merely just to keep it all under 80 columns without having to do line-breaks in the middle of a function call.

Honestly, it's merely just to keep it all under 80 columns without having to do line-breaks in the middle of a function call.
jordan commented 2023-09-21 13:59:31 +00:00
Owner
Copy link

Fair enough, I'm good with that. It doesn't look bad by any means, the code is still readable. Everyone has their own style and I have no real problems with this one.

Fair enough, I'm good with that. It doesn't look bad by any means, the code is still readable. Everyone has their own style and I have no real problems with this one.
jordan marked this conversation as resolved
jordan reviewed 2023-09-20 19:57:51 +00:00
src/Routes/RouteAdminDeactivate.c
@ -0,0 +96,4 @@
if (method == HTTP_DELETE)
{
char * str;
jordan commented 2023-09-20 19:57:51 +00:00
Owner
Copy link

Again, why use an intermediate variable here?

Again, why use an intermediate variable here?
lda marked this conversation as resolved
jordan reviewed 2023-09-20 19:59:36 +00:00
src/include/User.h
@ -205,0 +207,4 @@
* .Fn UserDeactivate ,
* but saves the reason and admin that deactivated the user
*/
extern int UserDeactivateWithInfo(User *, char *, char *);
jordan commented 2023-09-20 19:59:36 +00:00
Owner
Copy link

Is a separate function really necessary? Shouldn't we just modify UserDeactivate() to take the additional params?

Is a separate function really necessary? Shouldn't we just modify `UserDeactivate()` to take the additional params?
lda commented 2023-09-21 06:13:49 +00:00
Author
Contributor
Copy link

Considering how easy refactoring the code to use the proper UserDeactivate signature(which, to be frank, I should have done sooner), I think it's fine to change a few lines.

Considering how easy refactoring the code to use the proper `UserDeactivate` signature(which, to be frank, I should have done sooner), I think it's fine to change a few lines.
lda marked this conversation as resolved
jordan commented 2023-09-20 20:02:18 +00:00
Owner
Copy link

This looks pretty good. I will read the code more closely and test it when I have time. In the meantime, take a look at my comments and see what you feel is relevant. If you disagree with something, that's fine, we can certainly discuss it!

This looks pretty good. I will read the code more closely and test it when I have time. In the meantime, take a look at my comments and see what you feel is relevant. If you disagree with something, that's fine, we can certainly discuss it!
lda added 1 commit 2023-09-21 06:08:57 +00:00
lda added 1 commit 2023-09-21 06:24:56 +00:00
jordan reviewed 2023-09-25 13:37:21 +00:00
src/Routes/RouteDeactivate.c
@ -127,3 +127,3 @@
}
if (!UserDeleteTokens(user, NULL) || !UserDeactivate(user))
if (!UserDeleteTokens(user, NULL) || !UserDeactivate(user, NULL, NULL))
jordan commented 2023-09-25 13:37:21 +00:00
Owner
Copy link

I think we should set these parameters instead of having them be NULL. The from should be the user, and the reason can just be hard-coded if necessary.

I think we should set these parameters instead of having them be `NULL`. The `from` should be the user, and the `reason` can just be hard-coded if necessary.
jordan commented 2023-09-25 13:38:42 +00:00
Owner
Copy link

Oh nevermind, I see that the default behavior of this function is to set it to the user's own name.

Oh nevermind, I see that the default behavior of this function is to set it to the user's own name.
jordan marked this conversation as resolved
jordan merged commit cb41716bf3 into master 2023-09-25 13:39:24 +00:00
jordan referenced this pull request from a commit 2023-09-25 13:39:25 +00:00
Implement #27 (#36)
lda referenced this pull request from a commit 2023-09-25 20:41:23 +00:00
Implement #27 (#36)
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Telodendria/Telodendria#36
No description provided.
Delete branch "lda/telodendria:implement-27"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?