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

Cache test case #79

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

Cache test case #79

wants to merge 15 commits into from

Conversation

SagarBajpai
Copy link
Contributor

@SagarBajpai SagarBajpai commented Apr 10, 2021

Written Unit test for the Cache clearing page.

Cases covered for now are :

  1. should show last time user cleared the cache
  2. Clear cache button should get disabled on clicking it
  3. Below button should show the total number times user has already cleared the cache that day
  4. Clear cache button should be disabled if all user has already cleared cache 3 times

More tests will add to this file.

Run this module by running the test suite: npx ember test --server --filter="Integration | Component | self-clear-cache"

Closes #67

package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@swarajpure swarajpure left a comment

Choose a reason for hiding this comment

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

Can we use Ember Test Selectors instead of regular querySelector? More in comments

tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
ember-cli-build.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@ankushdharkar ankushdharkar left a comment

Choose a reason for hiding this comment

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

@SagarBajpai Missing a test where the button click was checked. Can we add that please?

tests/integration/components/self-clear-cache-test.js Outdated Show resolved Hide resolved
@SagarBajpai SagarBajpai requested a review from ankushdharkar May 13, 2021 17:33
@oneonone97 oneonone97 mentioned this pull request May 21, 2022
import { hbs } from 'ember-cli-htmlbars';

const EPOCH_TIMESTAMP = 1626242030;
const HUMAN_TIME = ' July 13, 2021 10:53:50 PM IST';

Choose a reason for hiding this comment

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

Is this a feature? Having space before the text?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed that

Copy link
Contributor

Choose a reason for hiding this comment

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

Good eye @pallabez.
How does the component handle timezones though? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

In tests, we can get the time using the Date object with our provided epoch time and while implementing the component, we can do the conversion using the Date object in a similar way, It converts epoch time according to the user's current timezone

Copy link
Contributor

Choose a reason for hiding this comment

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

let myDate = new Date(1656237389*1000);

assert.verifySteps([CACHE_CLEAR_CLICKED]);
});

skip('it disables button after a click', async function (assert) {

Choose a reason for hiding this comment

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

On what condition it gets enabled again?

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same doubt, need to ask @ankushdharkar, but probably there will be a fixed time after which the button gets enabled again and it will be handled on the frontend, we need not to test that I guess

Choose a reason for hiding this comment

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

How I think it should be -
Button clicked > Handle Click function calls which disables button & an API request to clear cache > Wait for server to respond > Server responds > Enable button again

Copy link
Contributor

@ankushdharkar ankushdharkar Jun 25, 2022

Choose a reason for hiding this comment

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

Good questions and suggestions.

We should be testing this on Frontend AND Backend. Backend should always validate and verify the behaviour (otherwise a bad user can just use curl/postman/etc to trigger the API anyway)

The button to be disabled here is to prevent multiple calls from firing (because clearing cache costs us as well as, it will cost crypto to clear your cache in future). We want to ideally make idempotent calls to clear the cache, and we should discuss this. We would have to implement some sort of UUID for this.

You can read more about the solution here: https://stripe.com/docs/api/idempotent_requests, but until then, we should just do a simpler implementation of maybe disabling the button for 5 seconds and move on.

  • Start an RFC for the the idempotent requests setup

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

Tests for clearing cache by member
7 participants