imgiitroorkee / omniport-backend Goto Github PK

The entire Omniport project, both the Django app and the Dockerised distribution, is scattered with README.md files spread across directories. The goal is to standardise a one-liner about each repository and a structure for these README.md files so that all these files are cohesive and meaningful as a quick-help alternative to the more comprehensive documentation being planned in #12 which is an issue that is coupled with but independent from this one.

Installed packages from requirements.txt and then tried running python manage.py runserver and returns errors. I think it would be great for anyone if the setup for development environment is this simple.

Commit 77678eb introduced rudimentary support for integrations via parsed YAML. This is in its infancy and so far supports only Sentry as an optional integration.

The goal is to extend this integration system into a full-fledged plugin system allowing developers to develop integrations for Omniport (with various third-party software) and unlock new functionality.

The mission is to extract all such elements as root Model and root ModelSerializer into a Formula 1 of the backend. This would allow other services to import such elements from Formula 1 instead of the kernel which must change really rarely in production due to delicate swappable models.

This is a long, tedious and complicated task so long-term dedication is a must before taking this on.

Django Permanent used to work for us. It provided us with removed and masked our querysets to filter out deleted objects. But it has run its course because of its use of a DateTimeField for storing whether an object has been deleted (quirky but acceptable) and its lack of integration with Django Admin.

The goal is to switch packages to Django Reversion, which offers more features, is more active and has more stars and contributors on GitHub. This would tie in with #23 because coupling with Django Admin invariably implies writing ModelAdmin classes.

Because style guidelines should focus more on the readability than forcibly enforcing the rules.

It is a very minor issue but I feel closing these opened file streams would be better.

https://github.com/IMGIITRoorkee/omniport-backend/blob/master/omniport/core/discovery/discovery.py#L80

Over time it seems like requirements.txt has gathered a lot of unnecessary packages. Also #11 points out that there are multiple levels of dependencies like production and development.

The goals is to purge the requirements.txt file of no-longer-needed dependencies and to split it into a set of files based on a two-tier system. This would also result in faster Docker container build times and leaner production images so everyone stands to benefit.

Since Omniport is an app that is the lifeline of the campus, anything going wrong must immediately be reported, traced, identified and fixed.

Goals:
To setup a mechanism for efficient logging with structured, app-wise logs including log rotation and standard multiple log levels.

An app comes with assets that are akin to static files, in the sense that these are just icons and other preloaded images. But these assets are not technically a part of static/ which are more suited for serving CSS and JS files. In other words, they are like a designer's static files.

The goal is to set up Discovery and Configuration to locate these assets and serve them both in development and in production similar to the way static files are collected and served. Extending or hijacking the Django static files flow is recommended.

Describe the bug
Backend throws a TypeError if integrations is left blank in base.yml.

This is because in

_CONF.integrations is None and hence not iterable here.

To Reproduce
Make integrations blank in base.yml

Expected behavior
There should be a check first, to see if integrations is None or not, to avoid this TypeError

Operating System:

• OS: Linux
• Browser: Firefox

Want to take up?
Sure

We are facing the issue described here: Kozea/WeasyPrint#915 currently due to mismatch in the version of the docker base image, GTK cairo, and the Weasyprint.

On using the command,
./script/start/django.sh -p <port_number>
The server gets started even if the port_number is outside the range of 60000 to 60031!

The models in kernel and shell are, as of now, being directly registered in the admin site, which leads to less than optimal forms being rendered. The goal is to write ModelAdmin classes, using features like inlines, change_form, add_form etc to make a highly functional admin site.

Is your feature request related to a problem? Please describe.
Many pull requests to the omniport ecosystem have mistakes in formatting and more than half of the comments are about the same.

Describe the solution you'd like
Integrate a code formatter as a management command. Further, we can also add it as git hooks to other repositories.

Describe alternatives you've considered
Manually correct formatting - which is a pain and delays a bug fix or a feature addition

Want to take up?
In future but can surely review.

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: relation "kernel_student" does not exist
LINE 1: ...t_semester", "kernel_student"."current_cgpa" FROM "kernel_st...
                                                             ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 35, in inner
    response = get_response(request)
  File "/omniport/omniport/middleware/person_roles.py", line 42, in __call__
    silent=False
  File "/omniport/core/kernel/managers/get_role.py", line 23, in get_role
    role = query_set.get(person=person)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 397, in get
    num = len(clone)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 254, in __len__
    self._fetch_all()
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 1179, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 53, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1067, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 100, in execute
    return super().execute(sql, params)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/site-packages/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: relation "kernel_student" does not exist
LINE 1: ...t_semester", "kernel_student"."current_cgpa" FROM "kernel_st...
 

We get this on running the server and visiting the admin site (/admin).

The LocationInformation generic in the Omniport kernel has been written keeping fundamental PostgreSQL in mind. This can be implemented much more optimally with PostGIS, which is the objective of this particular issue.

Describe the bug
A clear and concise description of what the bug is.
500 error when the user's IP address is not in allowed IP rings.

To Reproduce
Steps to reproduce the behavior:
Access an app outside it's allowed IP rings.

Expected behavior
A clear and concise description of what you expected to happen.
404 Not Found

Screenshots
If applicable, add screenshots to help explain your problem.

Operating System:

• OS: Linux
• Browser: Chrome

Additional context
Add any other context about the problem here.

Want to take up?
Are you interested in and capable of tackling this issue? [yes|no]

Is your feature request related to a problem? Please describe.
Last time, I had to use python shell separately to promote maintainers.

Describe the solution you'd like
Integrate it in already built scripts for students.

Want to take up?
😛

We use virtualenv (on off-container installations) and pip to install and manage dependencies. But there are clear problems with the way pip handles the requirements graph (one being the flat requirements.txt file). We've circumvented this problem with crude solutions to #11 and #20 but there is a better way.

Enter pipenv, a tool that manages the dependency graph and combines the power of pip and virtualenv. It introduces Pipfile and Pipfile.lock to manage the dependencies, much like Node.js. The goal is to use pipenv and make Omniport compatible with this new tool.

The mixins PeriodMixin and BlurryPeriodMixin use the start and end date very primitively. They might be replaced with a DateRangeField offered by the django.contrib.postgres package, which has very efficient range overlap functions (currently written by hand in the model).

The goal is to understand if DateRangeField suits all our usecases and if it does, rewrite the period mixins to use the more advanced method of dealing with dateranges. Even if it doesn't psycopg2.extras provides DateTimeTZRange and __contained_by lookup that can be used for more efficient checking of whether the range is past, current or due.

In order to move documentation to a more standardised and consistent format and later host it on a cloud documentation host like ReadTheDocs, one would have to rewrite a whole lot of docstrings in reStructuredText, install Sphinx and then push the docs to RTD. This is a good project if you are into writing technical documents and would like to have motivation to go through the entire codebase in detail. In short, the goal is to have documentation on ReadTheDocs.

Describe the bug
Regardless of what is passed in the query parameter, redirect_url, of the URL - https://internet.channeli.in/oauth/authorise/?client_id=<CLIENT_ID>&redirect_url=<REDIRECT_URL>, the app always gets redirected to the first URL which is registered in the Application model.

To Reproduce

1. You must have an app which is approved for OAuth.
2. Register multiple redirect_urls.
3. Pass any redirect_url which is not the first one.

Expected behavior
It should redirect to the URL which is specified in the redirect_url and is also registered in the redirect_uris field of Application model.

Operating System:

• OS: Ubuntu 20.04
• Browser: Chrome
• Version: 83.0.4103.116

Additional Context

• http://internet.channeli.in/oauth/authorise?client_id=[REDACTED]&redirect_url=http://127.0.0.1:8000/oauth&state=icwueifwueiocujweuilwiebrbilwbeirubeiwruiwerwouiwvuoaweifubi
  Even though the redirect_url here is http://127.0.0.1:8000/oauth&state=icwueifwueiocujweuilwiebrbilwbeirubeiwruiwerwouiwvuoaweifubi, it will still redirect it to the first URL which is registered.
• This is most likely a bug of oauth2_provider

Want to take up?
Yes, can mentor any contributor (most probably Project Associates 😄 )

Is your feature request related to a problem? Please describe.
A long standing issue in Django had a direct consequence on our project. Contenttype fields, entity_content_type and entity_object_id, (default names suggested by the Django framework) did not have a good widget attached to them so one had to manually query the database to find the instance ID in order to populate the fields.

In 2019, a PR was submitted and merged which added a workaround to select entity_content_type using a dropdown list and the entity_object_id from a popup window. Being a workaround, it also brought some limitations with it.

1. Under-the-hood JavaScript written to render the popup window only gets triggered when the fields are named entity_content_type and entity_object_id. This posed problems in apps like noticeboard where the field names are different - persona_content_type.
2. The code is not reusable which undermines the philosophy of Omniport.

Describe the solution you'd like
To convert this workaround into a concrete solution, I suggest making a Django Widget or a model field in a different repository (let's keep it under @IMGIITRoorkee only). Further, we can publish the package to PyPI. Eventually, solving both of the problems listed above.

The widget or the model field should add the following functionality:

1. Convert the entity_content_type (the name can by anything) field from a text field to dropdown list.
2. Clicking an option in the list should trigger a pop-up which would contain all instances of the selected model in contenttype.
3. The instances in the pop-up window should be paginated so that the request doesn't timeout.
4. It should also have a search functionality so that the maintainers do not spend too much time finding the instance they are looking for.
5. Support for adding custom styles to the widget. Not a priority at all.
6. Upon clicking an instance, the pop-up should close and write its primary key to the entity_object_id (again, name can be anything) field.

There can be more features or the implementation might be different but that is subjected to the discussion under this ticket.

Additional context
PRs related to the previous work on this.

1. IMGIITRoorkee/omniport-backend-formula-one#3
2. IMGIITRoorkee/omniport-backend-formula-one#5
3. #81

Want to take up?
Definitely up for reviewing or answering more questions.

This can be a point of discussion, should we return "Invalid credentials" or some more justified message.

Originally posted by @pradumangoyal in #127 (comment)

Is your feature request related to a problem? Please describe.

A single command, to setup any omniport app (along with all the dependent services) in local for development.

• Why this is required?
  For example, If someone wants to setup faculty-profile app locally, along with the respective Backend and Frontend repos, he/she also needs to clone two services i.e. common-biodata and faculty-biodata. But sadly, user gets to know about these dependent repos only by errors which occurs when he/she tries to migrate the database after cloning app's backend.

Describe the solution you'd like

Since every omniport app and service have a generic nomenclature, we can make a conf file for each app, which will contain names for all the dependent repos.

We can execute this script in codebase/ which will correspondingly,

1. Clone app's Backend and Frontend repos in respective directories.
2. Clone dependent services in respective directories.
3. Perhaps even migrate the database!

Describe alternatives you've considered

@algomaster99 suggested that,

1. We can have a something like python mange.py resolve-dependancy which we can execute in python shell itself.
2. Since it isn't necessary for cloning Frontend sometimes, we can make this optional and ask user if he/she wants to setup Frontend as well.

Additional context
This script would save time and efforts which goes into running git clone multiple times in different directories before migrating the app :-)

Want to take up?
yes

I see some of the packages which aren't probably required or may have been added due to pip freeze (if that's what has been done).

Configuration is another app that handles the loading and population of settings from various .yml configuration files. This app (much like discovery in #4 and #5) must be decoupled from the code-base and split into it's own standalone package and open-sourced because it is very versatile and flexible, and I can imagine many people would want something like it.

Over time we've had apps like ping-pong, protocol-tests and so on to ensure that the cogs in Omniport have been turning smoothly. This is a long-term, sustainable extension of that idea, in the sense that it is another tests app but it would cover a much broader range of tests:

• http(s) and ws(s) functionality (Hello World! over http(s) and Ping-Pong over ws(s))
• media files upload and delivery (simple image upload and render)
• static files upload and delivery (branding imagery)
• frontend-framework delivery (the React app itself)

and any more that come up in this thread. The app will have a corresponding React app for the frontend as well.

As of now, none of the database lookups have been in any way optimised. However Django provides a pages' worth of tools and techniques to optimise DB queries, which should be understood and implemented.

• Database access optimization
• Model index reference

The goal is to use them wherever misconduct is spotted and apply the fixes in a documented way as an example for other apps to be lead with.

Discovery is at the heart of Omniport. It is the app that largely makes the project the flexible, versatile and seamless package that it is. So I can imagine there would be tons of takers were this app to be flexible and generalised enough to support any Django app and standardised for any Django app to be able to plug it in. Even if not, decoupling it from Django settings would allow us to keep the codebase very clean and modular so it's a win-win.

Currently Django settings have been divided into two files: base.py and settings.py based on whether the values are hard-coded or loaded from YAML configuration files. This seemed like a good idea at the time but now not so much.

The goal is to split the settings into a bunch of chunk-sized settings files, say integrations.py for loading settings pertaining to 3rd party integrations like Sentry and Firebase, and services.py for loading those pertaining to databases and message brokers and so on. Finally a settings.py that imports all of these and an __init__.py that imports from settings.py.

This would bring much order to the chaos.

Currently the auth views use DRF only as a namesake, making use of the APIView class. This can be vastly enhanced by the use of serialisers, viewsets and other DRF magic.

Goals:
Thoroughly study DRF and implement auth in the most idiomatic way possible.

Write short and succinct hints to PyPI packages in INSTALLED_APPS to avoid confusion as to what their purpose is.

For example,

INSTALLED_APPS = [
    ...
    # PyPI packages
    'rest_framework',  # For writing JSON-backed APIs in Django
    ...
]

Try not to cross the 80-character limit

The goal is to update the datetime_modified on the model SessionMap in accordance with which session_key was used to authenticate any given request. This will ensure that every session has the accurate 'last seen' datetime.

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Operating System:

• OS: [e.g. iOS]
• Browser [e.g. chrome, safari]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

Want to take up?
Are you interested in and capable of tackling this issue? [yes|no]

Describe the bug
There isn't any unique field in the FacultyMember model currently. There should be a unique field throughout the model which can be used to identify one Faculty from the list. I think employee_id can be one such field, as added in the commit in shell for IIT-R.

Currently the module shell is very deeply coupled with the rest of omniport-core. This is not good because shell is the layer of customisation for India and IIT Roorkee in particular.

Goals:
Decouple shell into a repository of its own in such a way that the project is fully usable without it and properly customised with it.

Is your feature request related to a problem? Please describe.
The support for custom URL will help in always processing the token through the app and not send a request to the server directly. This was done right in the older channeli OAuth.
The current Omniport OAuth does not allow setting up a redirect URL with a custom schema.

Describe the solution you'd like
Update the URLValidator here?

Describe alternatives you've considered
Going with https as schema but that may send the request to the server directly and may not serve the purpose completely.

Want to take up?
no

Currently, there is a skip authorisation boolean field in the Application model in OAuth, but the feature is not implemented. This feature can be useful for the apps by maintainers themselves, built on different tech stack but integrated with their Omniport Oauth.

Discovery runs of lists of dictionaries containing nested lists of dictionaries. This is a very rudimentary, non-sustainable approach. The goal is to move away from this structure to a more hierarchical one, based on object-oriented principles.

omniport-backend lags behind in terms of github community standards:

This can be improved by adding CODE_OF_CONDUCT, CONTRIBUTING, ISSUE_TEMPLATE, and PULL_REQUEST_TEMPLATE to the repository.

The services running in containers can at times misbehave while not crashing the container. This causes the container to malfunction without initiating a restart. The mission is to add healthcheck properties to all services.

Apps are as much a model of Omniport as any other, given their pluggable nature. Referencing the apps from other applications such as the feed and Helpcentre requires there to be some model-like features in the app system.

However the ability to add and remove them over time makes the use of conventional Django models quite problematic because every restart that runs Discovery and Configuration would trigger a long process of realigning the database with the current position of apps/ in the filesystem. A better alternative is to use the Django cache and write pseudo-models that are everything like a model except that they live in Redis rather than PostgreSQL making updates many times faster.

Some groundwork has already been laid out like primary key (namely the name key in nomenclature) and AppConfiguration objects. What's required is to complete this architecture.

We have written Discovery as a way to find and load apps into the Django ecosystem. However there is still a discrepancy as the AppConfig class has to explicitly state the name and verbose_name of the app. Also Django already provides a feature that strongly resembles a weaker Discovery. Mote info about this inbuilt registry can be found in the docs.

The mission is to integrate our AppConfiguration architecture with Django's.

Describe the bug
500 error on OAUTH /open_auth/get_user_data/

To Reproduce
Send a get request without Authorization header

Expected behavior
401 HTTP Response

Screenshots
NA

Operating System:

• OS: [Ubuntu 18.04 LTS]
• Browser [chrome]
• Version 79.0.3945.130 (Official Build) (64-bit)

Additional context
NA

Want to take up?
no

Is your feature request related to a problem? Please describe.
It will be nice to have a feature to skip the same authorization form when performing OAuth with Omniport for the nth time.

Describe the solution you'd like
An option for the client in the developer service whether to enforce this authorization form every time their user tries to log in or not.

Describe alternatives you've considered
Follow the documentation here.

Additional context

Want to take up?
👍

Override Django's startapp management command to add more functionality, a non-exhaustive set of which includes:

• obtain a template from GitHub, created by the person taking this issue up
• create the folder in apps or services based on the type chosen when invoking the command
• create a decently populated config.yml file
• create a URLconf in the Omniport style, i.e. http_urls.py and ws_urls.py
• create modules for models, views, serializers and consumers rather than files