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

LINE Messenger Setup Notification has wrong fill-in description. #4900

Closed
1 task done
SamWang8891TW opened this issue Jul 3, 2024 · 6 comments
Closed
1 task done
Labels
area:notifications Everything related to notifications bug Something isn't working question Further information is requested

Comments

@SamWang8891TW
Copy link

SamWang8891TW commented Jul 3, 2024

📑 I have found these related issues/pull requests

There are no related issue reported as far as I can tell.

🛡️ Security Policy

Description

The description of "Channel access token" should be the description for "User ID" and vice versa. See picture below for detailed information.
IMG_A47967EF2EFC-1

👟 Reproduction steps

Just go to settings(from top right icon) > Notifications > Setup Notification > Select LINE messenger.
From there you can see what am I talking about.

👀 Expected behavior

The description of "Channel access token" should be the description of "User ID" and vice versa.

😓 Actual Behavior

They are placed opposite.

🐻 Uptime-Kuma Version

1.23.13

💻 Operating System and Arch

Server: Debian GNU/Linux 12 (bookworm) aarch64

🌐 Browser

MacOS Safari 17.4.1 (19618.1.15.11.14)

🖥️ Deployment Environment

  • Runtime: Docker version 27.0.3
  • Database: I'm only running on docker container and I really don't know.
  • Filesystem used to store the database on: Raspberry Pi 4 8G eMMC
  • number of monitors: 7

📝 Relevant log output

No response

@SamWang8891TW SamWang8891TW added the bug Something isn't working label Jul 3, 2024
@CommanderStorm
Copy link
Collaborator

Do you want to get the credit for this or should I submit a PR?
The file that would need changing is https://github.com/louislam/uptime-kuma/blob/master/src/components/notifications/Line.vue

@CommanderStorm CommanderStorm added question Further information is requested area:notifications Everything related to notifications labels Jul 3, 2024
@SamWang8891TW
Copy link
Author

@CommanderStorm Huge thanks! After scanning through the code I think all I need to do is to change the two texts, but I still want to test it just to make sure. Since this is the first time me do pull request, I'm a bit nervous. I really wanna do it myself at least once but since my father is going to a surgery and I'm staying in the hospital, you might not able to see the pr immediately.

I'll do my best and if I've done something wrong please let me know and I'll fix it.

Cheers!

@SamWang8891TW
Copy link
Author

SamWang8891TW commented Jul 5, 2024

@CommanderStorm So after messing around with the non-docker option just to test my code for two days, I cannot get further progress. After I ran node server/server.js The webpage keeps redirecting me from root to /setup-database and gave me page not found. I have two questions to ask if you are willing to help me, if u find helping me to learn this stuff too difficult you can just to pr, but I really wanna learn this.

I appreciate for your help!

  1. If I modify the files as you mentioned, it means that I have to use the non-docker method to test it right?

  2. As I said above, I cannot get it tested. The logs are below:

    Welcome to Uptime Kuma Your Node.js version: 18.13.0 2024-07-04T15:24:49+08:00
    �[SERVER] INFO:�Env: production 2024-07-04T15:24:50+08:00
    �[SERVER] INFO:�Uptime Kuma Version: 1.23.13 2024-07-04T15:24:50+08:00
    �[SERVER] INFO:�Loading modules 2024-07-04T15:24:51+08:00
    �[SERVER] INFO:�Creating express and socket.io instance 2024-07-04T15:24:51+08:00
    �[SERVER] INFO:�Server Type: HTTP 2024-07-04T15:24:51+08:00
    �[SERVER] INFO:�Data Dir: ./data/ 2024-07-04T15:24:51+08:00
    �[SETUP-DATABASE] INFO:�db-config.json is not found or invalid: ENOENT: no such file or directory, open 'data/db-config.json' 2024-07-04T15:24:51+08:00
    �[SETUP-DATABASE] INFO:�Starting Setup Database on 3001 2024-07-04T15:24:51+08:00
    �[SETUP-DATABASE] INFO:�Open http://localhost:3001 in your browser 2024-07-04T15:24:51+08:00
    �[SETUP-DATABASE] INFO:�Waiting for user action... 
    

@CommanderStorm
Copy link
Collaborator

  1. For just swapping strings, I would gennerally not expect you to set up a dev environment ^^

    If you still want to test how the change looks without doing so, you can see our recomendation for testing PRs

  2. If you still want to setup your environemnt
    The Installation and contribution guide is slightly different.
    For contribution, you would run npm run dev instead

    See https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md how to get a dev environment set up.

@SamWang8891TW
Copy link
Author

@CommanderStorm Thanks! By using npm run dev I am able to setup the dev environment to check. But things turned out pretty interesting.

The reason I really wanna run the code even it is just swapping strings is because I was suspecting that the code is correct and the issue only happens in docker image, and turns out I'm right.

In https://github.com/louislam/uptime-kuma/blob/master/src/components/notifications/Line.vue the order of the two strings were and are correct, and I was able to get the correct output by running npm run dev without modifying the strings. But the docker image has wrong string order.

So I connected the console of the docker container and looked for Line.vue, and sure enough the order of the two strings is opposite comparing to https://github.com/louislam/uptime-kuma/blob/master/src/components/notifications/Line.vue .

It is REALLY strange that the file in docker container doesn't match the file on github since there is no pr or issue about this one. Is there any method that I can contribute to fix the docker image?

The screenshot of how the Line.vue looks in docker container:
image

@CommanderStorm
Copy link
Collaborator

True, closing as resolved.
Forgot about #4527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:notifications Everything related to notifications bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants