Initial support for #686. #705

Open
fdcastel wants to merge 3 commits from fdcastel/push-api-tags into master
fdcastel commented 3 years ago (Migrated from github.com)
Owner

Related: https://github.com/louislam/uptime-kuma/issues/686

Initial implementation. Please advice.

Done:

  • All query params sent in /api/push/ except for msg and ping are considered tags to update;
  • Non-existing tags are ignored;
  • Do not put additional database pressure if no tags are passed.

To fix:

  • When using trx.dispense() the returning Bean fails on bean.toJSON() call. Using R.dispense() for now;
  • What callback should be sent to the UI for updating tag values? Currently the code is updating the database but the new tag values are only updated after a page refresh 😢 .
Related: https://github.com/louislam/uptime-kuma/issues/686 Initial implementation. Please advice. Done: - All query params sent in `/api/push/` except for `msg` and `ping` are considered tags to update; - Non-existing tags are ignored; - Do not put additional database pressure if no tags are passed. To fix: - When using `trx.dispense()` the returning `Bean` fails on `bean.toJSON()` call. Using `R.dispense()` for now; - What callback should be sent to the UI for updating tag values? Currently the code is updating the database but the new tag values are only updated after a page refresh 😢 .
fdcastel commented 3 years ago (Migrated from github.com)
Poster
Owner

Rebased with latest master.

Rebased with latest master.
chakflying commented 3 years ago (Migrated from github.com)
Owner

Previously I relied on monitorList to update tags because of infrequent use. I guess we can add a new event in /src/mixins/socket.js specifically for updating tags?

Previously I relied on `monitorList` to update tags because of infrequent use. I guess we can add a new event in `/src/mixins/socket.js` specifically for updating tags?
Saibamen commented 3 years ago (Migrated from github.com)
Owner

Please add Related: https://github.com/louislam/uptime-kuma/issues/686 to your first message

Issue in title is not clickable
And adding URL in first message will also connect your PR with issue in right sidebar

Please add `Related: https://github.com/louislam/uptime-kuma/issues/686` to your first message Issue in title is not clickable And adding URL in first message will also connect your PR with issue in right sidebar
fdcastel commented 3 years ago (Migrated from github.com)
Poster
Owner

Rebased with latest master. Support for duplicated tags (updates the oldest one).

Rebased with latest master. Support for duplicated tags (updates the oldest one).
fdcastel commented 3 years ago (Migrated from github.com)
Poster
Owner

Why beans returned from R.findOne(), R.dispense(), etc. are different from the ones returned from transactions? (trx.findOne(), trx.dispense(), ...)

With the latest rebase with master, this PR has now two occurrences of this problem. I have to use R. instead of trx. to access .toJSON()

I'm new to this codebase. Please advice.

Why beans returned from `R.findOne()`, `R.dispense()`, etc. are different from the ones returned from transactions? (`trx.findOne()`, `trx.dispense()`, ...) With the latest rebase with master, this PR has now two occurrences of this problem. I have to use `R.` instead of `trx.` to access `.toJSON()` I'm new to this codebase. Please advice.
louislam commented 3 years ago (Migrated from github.com)
Owner

I would suggest that it should be implemented without transaction at this moment.

I found that transaction is stable only if it is a raw query.

I would suggest that it should be implemented without transaction at this moment. I found that transaction is stable only if it is a raw query.
fdcastel commented 3 years ago (Migrated from github.com)
Poster
Owner

Rebased with latest master. Removed transaction support.

What callback should be sent to the UI for updating tag values?

Rebased with latest master. Removed transaction support. What callback should be sent to the UI for updating tag values?
This pull request can be merged automatically.
You are not authorized to merge this pull request.
Sign in to join this conversation.
Loading…
There is no content yet.