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

See current pinger scheduled notificactions #1

Open
dmitrijs-balcers opened this issue May 28, 2024 · 28 comments
Open

See current pinger scheduled notificactions #1

dmitrijs-balcers opened this issue May 28, 2024 · 28 comments

Comments

@dmitrijs-balcers
Copy link
Contributor

Would it be somehow possible to get API to acquire all scheduled pinger notifications for given user?
I am thinking that perhaps it would be possible to send a link in email through which it would be possible to access all the scheduled pinger notificaitons to review, edit and delete through some easy to use interface.
If you think it is possible I might be able to help out with coding FullStack solution or only UI if needed.

@MatissJanis
Copy link
Member

👋 It would definitely be a nice feature to have. I can try taking a look at building out the API sometime soon™️.

The API is located here: https://github.com/brokalys/sls-api

@MatissJanis
Copy link
Member

MatissJanis commented May 31, 2024

Ok, the API is now ready. brokalys/sls-api@cfbafaa

Here's the staging playground you can use: https://h4iamlt72f.execute-api.eu-west-1.amazonaws.com/staging/

Example query:

query {
  pingers(
    id: "4a9459aa-7eb9-11eb-b2a8-663c33f40218"
    unsubscribe_key: "1b27467e016c52b27d53ed06aadf1ef512bdb521"
  ) {
    results {
      id
      email
      category
      type
      price_min
      price_max
      price_type
      region
      rooms_min
      rooms_max
      area_m2_min
      area_m2_max
      frequency
      comments
      marketing
      created_at
      unsubscribed_at
      unsubscribe_key
    }
  }
}

HTTP headers:

{
  "X-Api-Key": "g2G4jwUchM3wH1ozVqr8SaY5m7FZkrjH2AsN9qOY"
}

Provide the pinger ID and the unsubscribe key. The result will contain an array of pingers for this specific user. Nothing will be returned if the ID/key does not match any pingers.

Prod will have the same syntax. API endpoint: https://api.brokalys.com ; key: tbxck3bRYU2ZdZFJi5Jtd7ycKZ4rBgIQ3XEYFDPj

--

There is no way to perform UPDATE operations. However, you can run the unsubscribePinger() and then createPinger() operations to mimic the same behavior.

@dmitrijs-balcers
Copy link
Contributor Author

That was quick! I got a bit swamped at work, will try to get to this!

@dmitrijs-balcers
Copy link
Contributor Author

All good on staging thanks!

  1. I tried to fetch pingers on prod with id and unsubscribe_key with the url parameters I derived from unsubscribe link I received in my email, but received following error:
{
  "errors": [
    {
      "message": "An unexpected error occurred. Please try again later."
    }
  ],
  "data": null
}

Could you please check in logs what I'm doing wrong? I did set prod X-Api-Key you provided, and for:

pingers(
    id: "4a9459aa-7eb9-11eb-b2a8-663c33f40218"
    unsubscribe_key: "1b27467e016c52b27d53ed06aadf1ef512bdb521"
  )  { ... }

this returns OKish data (not error):

{
  "data": {
    "pingers": {
      "results": []
    }
  }
}

I suspect that API-Key is Access limited perhaps?

  1. The selected area on the map, would I receive it through region value that I could further illustrate on google maps integration?

@MatissJanis
Copy link
Member

  1. patch has been deployed; try again now
  2. yes region can be used for that

You can use polygonStringToCoords to convert the string to a polygon. And then just draw the polygon on the map.

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jun 3, 2024

Thanks, works!

But interestingly now I receive a lot of null values and all of the pingers on prod for my email have null region:

{
          "id": "<id>",
          "email": "<email>",
          "category": "HOUSE",
          "type": "SELL",
          "price_min": 1,
          "price_max": 300000,
          "price_type": "TOTAL",
          "region": null,
          "rooms_min": null,
          "rooms_max": null,
          "area_m2_min": null,
          "area_m2_max": null,
          "frequency": "DAILY",
          "comments": null,
          "marketing": true,
          "created_at": "1679843489000",
          "unsubscribed_at": null,
          "unsubscribe_key": "<uns_key>"
        },

I think in pinger region should be always present right? Unless user somehow selected nothing on the map and we allow that on API validation?

@MatissJanis
Copy link
Member

You are correct! It's been a while since I worked with the pinger codebase, so I'm a bit rusty. The region field is now patched.

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jun 8, 2024

@MatissJanis I'm sorry to disturb again 🙈 I just returned to this.
it seems that in both staging and prod, I'm receiving every result as duplicate, regions match in every record.

@MatissJanis
Copy link
Member

Ha.. that's quite unfortunate. I'm OOO right now. Will try to pick this up again in a few weeeks.

@MatissJanis
Copy link
Member

The patch has been deployed. Thanks for the report!

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jun 23, 2024

@MatissJanis I faced another issue with unsubscribe. It seems to me that unsubscribe here doesn't work:
https://unsubscribe.brokalys.com/ at least I opened one of the Atrakstīties links, pressed unsbscribe, received unsubscribed successfuly on UI, but pinger still is returned in common list of pingers.

{"data":{"unsubscribePinger":true}}

Lol, I brainfarted, BE returns all pingers, even unsubscribed, I can just filter using unsubscribed_at

@dmitrijs-balcers
Copy link
Contributor Author

@MatissJanis plz rev: #2

@MatissJanis
Copy link
Member

Sorry for the slow review. My other side-project - Actual budget - recently got a huge influx of users, so that took much of my attention.

@MatissJanis
Copy link
Member

Ok, your PR is now deployed. I think the only remaining piece is to add a new link to the pinger emails we send out. I'll try to do that when I get some time (unless you beat me to it).

sls-pinger repository

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jul 15, 2024

Hmm I somehow can not get it working on prod.
For some reason https://pinger.brokalys.com/#/<id>,<unsub_key> is trying to fetch pingers from staging:

https://h4iamlt72f.execute-api.eu-west-1.amazonaws.com/staging/

@MatissJanis
Copy link
Member

Patched that now :)

@dmitrijs-balcers
Copy link
Contributor Author

It works!!!

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jul 23, 2024

well OK, sometimes throws this 🤔

I suppose it is this one:

console-breadcrumbs.js:33 Google Maps JavaScript API error: OverQuotaMapError
https://developers.google.com/maps/documentation/javascript/error-messages#over-quota-map-error

Perhaps there is some way to optimise this with some caching.

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jul 23, 2024

And another issue. Somehow I got this, without pressing any unsubscribe button 🙈 I was just refreshing the page.

image

Ohh, actually I did some updates of some of the records, but I saw the updated values on submit, so it must've updated on db instead of only unsubscribing. Will try to check a bit later what exactly server returns, as I'm filtering results on client.

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Jul 25, 2024

@MatissJanis how does it look on your end with Google Maps API usage?
I'm thinking that perhaps I can somehow cache maps as images using html2canvas on first render, and keep it in map with polygon as key.
Do you think there is need for such mechanism? Because after few refreshes we are going to get that "Something Went Wrong" view, especially if more users will use this.
Or am I the only one using pingers? 😄

For context, I'm talking about issue here #1 (comment), second issue I'm still monitoring, I'm not sure how did that happen.

@MatissJanis
Copy link
Member

Hmm.. the current quota is set to 6 loads per-minute, per-user. Which is quite low if we're loading multiple maps on a single page.

What if we instead used static maps on the pinger list page? That would rise the quote to thousands of requests and thus solve the issue.

https://developers.google.com/maps/documentation/maps-static/overview

@dmitrijs-balcers
Copy link
Contributor Author

Will check prolly over the next week 👍

@dmitrijs-balcers
Copy link
Contributor Author

@MatissJanis I still experience pingers being unsubscribed without me requesting for it at some seemingly random times. Could there be any automatic mechanism on backend that would trigger unsubscription based on some rules?

@MatissJanis
Copy link
Member

Can you give me one of the pinger IDs? I can try to check what's going on then.

@dmitrijs-balcers
Copy link
Contributor Author

dmitrijs-balcers commented Aug 7, 2024

Can you please check this one c7582af7-53fd-11ef-81ac-86739c9bd374? I created it, opened common view, clicked on map then closed without adjusting (updating) and it dissappeared.

@MatissJanis
Copy link
Member

MatissJanis commented Aug 8, 2024

When the "Apstiprināt izmaiņas" button is pressed - a unsubscribe API call gets triggered. So there must be a bug somewhere in brokalys/sls-pinger#2

Edit: my bad, that is expected because we don't do mutations for pingers. We unsubscribe and re-subscribe.

There is no code that would automatically unsubscribe pingers.. Any chance you're able to reliably reproduce the issue somehow? Opening the edit modal didn't cause the issue for me.

@dmitrijs-balcers
Copy link
Contributor Author

Any chance you're able to reliably reproduce the issue somehow? Opening the edit modal didn't cause the issue for me.

Absolutely have no idea, it is as if it happens exclusively when I'm not looking for this issue 😂
I think I will proceed with this, perhaps while working on it, something will catch my eye

@dmitrijs-balcers
Copy link
Contributor Author

Hi @MatissJanis this PR should be ready for review 🙏 #3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants