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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration with Django #206

Merged
merged 76 commits into from
Jul 28, 2023
Merged

Integration with Django #206

merged 76 commits into from
Jul 28, 2023

Conversation

RafaelHGOliveira
Copy link
Contributor

I made an integration with django that doesn't interfere with the code or with the tables already created in the database.

An auxiliary table is created to insert the new data, and it is possible to perform queries and CRUDs by Django Admin.

@RafaelHGOliveira
Copy link
Contributor Author

Moved to ./django/ in the last push

If query is needed, you can always run an external database and connect directly to it along with the bot. For any other direct bot interactions and callbacks, I can create a much less complicated REST API/webhook (Express) or Message Queue (AMQP) System.

I like the feature, but it's just adding too much complexity to the codebase without knowing if other users will actually use this. As always, if @Sayrix is okay with this, we can add it.

The way I did it, I don't think it's the best, but since I didn't want to interfere with the bot's operation and I don't have much knowledge in javascript, I ended up doing this "workaround" lol

Copy link
Owner

@Sayrix Sayrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you sync the branch, i think there are conflicts with some changes that we made

@RafaelHGOliveira
Copy link
Contributor Author

I had changed some files by mistake, now they must be with the changes you made.

Copy link
Owner

@Sayrix Sayrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also move the readme of django to DJANGO.md ?

@RafaelHGOliveira
Copy link
Contributor Author

RafaelHGOliveira commented Jul 28, 2023

Moved the django readme into the Django folder

@RafaelHGOliveira RafaelHGOliveira marked this pull request as ready for review July 28, 2023 19:02
@zhiyan114 zhiyan114 added this to the 5.2.0 milestone Jul 28, 2023
Copy link
Collaborator

@zhiyan114 zhiyan114 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Hey, mind cleaning up .gitignore a bit?
    I feel like not all of the files/directories that's listed will be generated when running the Django.
    If I'm wrong, let me know.
  • DJANGO.md and django/README.md seems to be a duplicate file
  • django/trigger.py seems to have print("aaaaa"), not sure if you accidently left it there during development.
  • Both django/tickets/tests.py and django/tickets/views.py are incomplete. if no test will be written, you can go head and delete it.

@RafaelHGOliveira
Copy link
Contributor Author

  • Hey, mind cleaning up .gitignore a bit?
    I feel like not all of the files/directories that's listed will be generated when running the Django.
    If I'm wrong, let me know.

I was using the .gitignore python template, now it should be just the essentials

  • DJANGO.md and django/README.md seems to be a duplicate file

I removed the DJANGO.md

  • django/trigger.py seems to have print("aaaaa"), not sure if you accidently left it there during development.

This was a print used for debugging that I ended up forgetting to delete

  • Both django/tickets/tests.py and django/tickets/views.py are incomplete. if no test will be written, you can go head and delete it.

They are standard Django files, but as they won't be needed I removed them

Copy link
Collaborator

@zhiyan114 zhiyan114 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright we good, I'll go head and merge it.

@zhiyan114 zhiyan114 merged commit 942832a into Sayrix:main Jul 28, 2023
@zhiyan114 zhiyan114 mentioned this pull request Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants