OptiPie / tradingview-optimizer-extension

OptiPie is an open source strategy optimizer automation tool for TradingView written in js.
https://optipie.app
GNU General Public License v3.0
40 stars 22 forks source link

Show correct period in reports #15

Closed iljamaki closed 1 year ago

iljamaki commented 1 year ago

The Period is shown as 1m in every report regardless of the timeframe used in the optimization. A recent commit message suggests there was maybe an intent to fix this(?), but it still occurs at least at commit 0b49f6f39de253f5cc.

AtakanPehlivanoglu commented 1 year ago

Hello @iljamaki ,

I have recently tested this issue using the latest extension v1.0.2 and didn't face any issues you mentioned. Here are my reports regarding this test,

image

iljamaki commented 1 year ago

@AtakanPehlivanoglu Ok so that suggests this is something that depends on the environment. I also use version 1.0.2 and the issue exists there

image

As I said earlier, I also tried cloning the repository locally as I wasn't sure which commit version 1.0.2 was made of, but also the latest commit 0b49f6f39de253f5c had the same problem.

My browser is Chrome Version 112.0.5615.121 (Official Build) (64-bit) and the OS is Windows 11 Home 22H2. Is there any other information I can provide you with to help resolve the issue?

AtakanPehlivanoglu commented 1 year ago

@iljamaki Well that's interesting for sure,

Could you please run the script below in your Chrome console while you are on tradingview.com? This script is directly from the latest extension version and I'm not sure how this code block can differ from one environment/browser to another since I have already tested it with multiple devices(macOS/Windows)

My Chrome version is Version 112.0.5615.138 (Official Build) (64-bit)

This code block matches the first innerWrap and group with wildcard matching and looks for the value of the first element which is the time period and I'm not sure how this can differ since this querySelector directly being used on TradingView DOM.

var timePeriodGroup = document.querySelectorAll("div[class*=innerWrap] div[class*=group]")
if (timePeriodGroup.length > 0){
    strategyTimePeriod = timePeriodGroup[1].querySelector("div[class*=value]")?.innerHTML
}

https://github.com/OptiPie/tradingview-optimizer-extension/blob/main/script.js#L88

AtakanPehlivanoglu commented 1 year ago

One additional piece of information I would ask for, Which tradingview subdomain are you using? I have also tested and validated on tr,ru,pl subdomains and worked fine but maybe this could be also the issue, just for sanity check @iljamaki

iljamaki commented 1 year ago

@AtakanPehlivanoglu It outputs '1m' while 30m is selected: image I'm using the main domain "tradingview.com".

I can try to take another look into the element structure of the page if I find some time for it.

iljamaki commented 1 year ago

@AtakanPehlivanoglu Okay I looked into it a bit more and as you may already have guessed from the screenshot I pasted above, the code only works if the user has not set any favorite timeframes.

This seemed to work regardless although there may be better ways of doing it:

var timePeriodGroup = document.querySelectorAll("div[class*=innerWrap] div[class*=group]")
if (timePeriodGroup.length > 1){
    selectedPeriod = timePeriodGroup[1].querySelector("button[aria-checked*=true]")
    strategyTimePeriod = (selectedPeriod ? selectedPeriod : timePeriodGroup[1]).querySelector("div[class*=value]")?.innerHTML
}
AtakanPehlivanoglu commented 1 year ago

@iljamaki great catch, I also realized that from the screenshot. Imo your code works just fine for both cases(favorite and no favorite time period selection). I have just refactored it a little bit since I would prefer more readable code over less code like a single if statement.

var timePeriodGroup = document.querySelectorAll("div[class*=innerWrap] div[class*=group]")
if (timePeriodGroup.length > 1) {
    selectedPeriod = timePeriodGroup[1].querySelector("button[aria-checked*=true]")

    // Check if favorite time periods exist  
    if (selectedPeriod != null) {
        strategyTimePeriod = selectedPeriod.querySelector("div[class*=value]")?.innerHTML
    } else {
        strategyTimePeriod = timePeriodGroup[1].querySelector("div[class*=value]")?.innerHTML
    }
}

When we are aligned with this, I can open a new branch and merge/deploy new version for this

iljamaki commented 1 year ago

@AtakanPehlivanoglu This is your project so feel free to fix it the way you want, I'll be happy as long as it works.

AtakanPehlivanoglu commented 1 year ago

Update: New version v1.0.3 is pushed to the Chrome Store. Whenever it's approved by Chrome Store, new version will be available for use.

Special thanks to the @iljamaki and @Daemon0x00000000 for their contributions to the project.