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.

Sign up for GitHub

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

Jump to bottom

Imp kill mithril request #45

Closed
wants to merge 0 commits into from
Closed

Imp kill mithril request #45

wants to merge 0 commits into from

Conversation

vokimon
Copy link
Member

@vokimon vokimon commented May 30, 2024

Description

Subtitutes mithril.request by standard javascript fetch, removing last bit of mithril use in Tomatic.

Changes

  • Using fetch instead mitrhil.request
  • Using modular front-end error handling like in somrepresenta
  • Handling specific common errors like:
    • VPN down
    • Unauthorized (login required)
    • Forbiden (not enough permision)

Checklist

Justify any unchecked point:

  • Changed code is covered by tests. -> Api not covered, sorry
  • Relevant changes are explained in the "Unreleased" section of the CHANGES.md file.
  • That section includes "Upgrade notes" with any config, dependency or deploy tweek needed on development and server setups.
  • Changes on the setup process (development, testing, production) have been updated in the proper documentation

Observations

Please, review

  • Error conditions for every api request

How to check the new features

Deployed in Pebrotic

Deploy notes

To remove the dependency, remove the node_modules and make deploy.

@vokimon vokimon self-assigned this May 30, 2024
javikalsan
javikalsan reviewed Jun 27, 2024
View reviewed changes
tomatic/ui/src/services/api.js Outdated
import jsyaml from 'js-yaml'
import Auth from './auth'
import messages from './messages'

const debugApi = false
const debugApi = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason of debugging as default? Would be worth extract the flag to a configuration/env file?

javikalsan
javikalsan approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@javikalsan javikalsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are very clear and easy to understand, thank you so much and congratulations! long live mithril 🥼 !

@vokimon vokimon closed this Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javikalsan javikalsan javikalsan approved these changes

@Beatrigz Beatrigz Awaiting requested review from Beatrigz

Assignees

@vokimon vokimon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vokimon @javikalsan