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

[change] Allow relative paths #249

Closed
nemesifier opened this issue Jul 11, 2022 · 6 comments · Fixed by #266
Closed

[change] Allow relative paths #249

nemesifier opened this issue Jul 11, 2022 · 6 comments · Fixed by #266
Labels
bug Something isn't working enhancement New feature or request

Comments

@nemesifier
Copy link
Member

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.

@nemesifier nemesifier added bug Something isn't working enhancement New feature or request labels Jul 11, 2022
@nemesifier nemesifier added this to To do (general) in OpenWISP Contributor's Board via automation Jul 11, 2022
@nemesifier nemesifier moved this from To do (general) to To do (Python & Django) in OpenWISP Contributor's Board Jan 23, 2023
@BassCoder2808
Copy link

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
Copy link
Member

pandafy commented Apr 11, 2023

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'
  1. 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.
  2. Generate a default notification by using the create_notification management command from the shell
cd tests/
./manage.py create_notification
  1. 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
Copy link

@pandafy I am Working on this issue.

@pandafy
Copy link
Member

pandafy commented Feb 24, 2024

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
Copy link

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
Copy link
Member

pandafy commented Feb 28, 2024

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.

arup1221 added a commit to arup1221/openwisp-notifications that referenced this issue Mar 3, 2024
nemesifier added a commit that referenced this issue Apr 22, 2024
JS code to implement this change + one selenium test.

Closes: #249

---------

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
OpenWISP Contributor's Board automation moved this from To do (Python & Django) to Done Apr 22, 2024
nemesifier added a commit that referenced this issue Apr 22, 2024
JS code to implement this change + one selenium test.

Closes: #249

---------

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
4 participants