search

openwisp / openwisp-notifications

Notifications module of OpenWISP
https://openwisp.io/docs/stable/notifications/
GNU General Public License v3.0
41 stars 42 forks source link

[change] Allow relative paths #249

Closed nemesifier closed 7 months ago

nemesifier commented 2 years ago

It would be good to change the notification widget to allow the use of relative paths. This helps the notification links to work also on systems which are using multiple domain names, some times a domain is reachable only from a private network but when users click on the notifications they are taken to the main domain name, which is not what they want.

BassCoder2808 commented 1 year ago

Hi @nemesisdesign, I wanted to contribute to this issue if it is still open, I have installed the openwisp-notifications and the setup part is done for the same, I wanted the steps like how can I reproduce the issue based on which I can change the code

pandafy commented 1 year ago

Steps to replicate the issue

  1. Run the test project in this repository
  2. Edit the /etc/hosts file to point internal.openwisp.test and external.openwisp.test to 127.0.0.1 using the following command 
    sudo bash -c 'echo "127.0.0.1    internal.openwisp.test external.openwisp.test" >> /etc/hosts'
  3. Update the site objects at http://127.0.0.1:8000/admin/sites/site/ as following: image Note: Instead of deleting the existing site object, edit it to use external.openwisp.test.
  4. Generate a default notification by using the create_notification management command from the shell 
    cd tests/
./manage.py create_notification
  5. Open http://internal.openwisp.test:8000/admin/ and click on the notification in the notification widget

Actual result After clicking on the notification, the user to sent to the target object with external.openwisp.test domain. E.g.: http://external.openwisp.test:8000/admin/openwisp_users/organization/b150de21-65b2-4499-b1a8-3af78e8442b5/change/

Expected result After clicking on the notification, the URL should take the user to the target object with internal.openwisp.test domain. E.g.: http://internal.openwisp.test:8000/admin/openwisp_users/organization/b150de21-65b2-4499-b1a8-3af78e8442b5/change/

arup1221 commented 9 months ago

@pandafy I am Working on this issue.

pandafy commented 9 months ago

It is important to update the ALLOWED_HOSTS settings in tests/settings.py like below to replicate this issue

ALLOWED_HOSTS = ['internal.openwisp.test', 'external.openwisp.test', '127.0.0.1']

Thanks to @arup1221 for reporting this in the developer chat. 👏🏼

arup1221 commented 9 months ago

Thanks also, @pandafy, for allowing me to work on this project. I am currently figuring out whether this bug is related to notification generation and how to redirect it to http://internal.openwisp.test:8000/admin/openwisp_users/organization/<id>/change/."

pandafy commented 9 months ago

The internal.openwisp.test and external.openwisp.test hostname provided above are just for example. These hostnames could be anything depending on the installation.

To understand the issue better, check the HTML code of the rendered notification element in the UI

Screenshot from 2024-02-28 15-34-53

Here, an absolute URL is used for the device. We want to have the relative URL so that the URL works across different hostnames.

What's the catch?

The same logic is used for adding notification message in the emails. We want the emails to continue having absolute URLs, otherwise users would not be able to follow the URL to OpenWISP.