-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: Correcting the routes for my-session #3606
Conversation
@iamareebjamal All the tests related to |
Finally the Green Ticks ❤️ , @iamareebjamal Please review & merge |
Steps taken to fix it? |
Creation of test account which was removed in db |
Automate that in the tests please. Create an issue and a PR |
What about the solution I told in gitter?
…On Thu, 7 Nov, 2019, 13:24 Areeb Jamal, ***@***.***> wrote:
Automate that in the tests please. Create an issue and a PR
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3606?email_source=notifications&email_token=AKQMTLWOAN6HQ2VTKSYZQP3QSPCSRA5CNFSM4JJLTDS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDLRCBI#issuecomment-550965509>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKQMTLUG5AGUHDTW4CMCSXTQSPCSRANCNFSM4JJLTDSQ>
.
|
No, that is overkill |
What should be the solutions , create an account and delete it after and
then create a new in script ?
…On Thu, 7 Nov, 2019, 13:27 Areeb Jamal, ***@***.***> wrote:
No, that is overkill
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3606?email_source=notifications&email_token=AKQMTLWDOMIGBMJPXJAWYJLQSPC5TA5CNFSM4JJLTDS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDLRIWI#issuecomment-550966361>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKQMTLUXKEAY74WVH4W5VI3QSPC5TANCNFSM4JJLTDSQ>
.
|
No, try to login, and if it fails, create a new account |
@iamareebjamal we can atleast merge this and make the release for now.
…On Thu, 7 Nov, 2019, 13:33 Areeb Jamal, ***@***.***> wrote:
No, try to login, and if it fails, create a new account
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3606?email_source=notifications&email_token=AKQMTLU2TW36FCLPDT6RLXLQSPDWJA5CNFSM4JJLTDS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDLRXRY#issuecomment-550968263>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKQMTLT524ZJYUOTDUEDLK3QSPDWJANCNFSM4JJLTDSQ>
.
|
We can merge this but release will be made after event wizard fix |
Get a review from peers |
… On Fri, 8 Nov, 2019, 13:06 Areeb Jamal, ***@***.***> wrote:
Get a review from peers
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3606?email_source=notifications&email_token=AKQMTLQSQAY2VCRHGWB3UZ3QSUJI3A5CNFSM4JJLTDS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDO76JY#issuecomment-551419687>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKQMTLWHNEGS2JV7IBNTRS3QSUJI3ANCNFSM4JJLTDSQ>
.
|
@prateekj117 Update? |
@iamareebjamal Back online!!! Looking into it. |
We are using ES6 syntax its good to migrate.
…On Sun, 10 Nov, 2019, 16:16 Prateek Jain, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/controllers/my-sessions/view.js
<#3606 (comment)>
:
> - actions: {
- openProposalDeleteModal() {
- this.set('isProposalDeleteModalOpen', true);
- },
- deleteProposal() {
- this.set('isLoading', true);
- this.model.destroyRecord()
- .then(() => {
- this.transitionToRoute('my-sessions.index');
- this.notify.success(this.l10n.t('Proposal has been deleted successfully.'));
- })
- .catch(() => {
- this.notify.error(this.l10n.t('An unexpected error has occurred.'));
- })
- .finally(() => {
- this.set('isLoading', false);
- this.set('isProposalDeleteModalOpen', false);
- });
- }
+ @action
+ openProposalDeleteModal() {
+ this.set('isProposalDeleteModalOpen', true);
}
-});
+
+ @action
+ deleteProposal() {
+ this.set('isLoading', true);
+ this.model.destroyRecord()
+ .then(() => {
+ this.transitionToRoute('my-sessions.index');
+ this.notify.success(this.l10n.t('Proposal has been deleted successfully.'));
+ })
+ .catch(() => {
+ this.notify.error(this.l10n.t('An unexpected error has occurred.'));
+ })
+ .finally(() => {
+ this.set('isLoading', false);
+ this.set('isProposalDeleteModalOpen', false);
+ });
+ }
+}
+
@iamareebjamal <https://github.com/iamareebjamal> Just the changes in
view.hbs are required. Changes done by @kushthedude
<https://github.com/kushthedude> LGTM.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3606?email_source=notifications&email_token=AKQMTLVA2XCW3EQCPD5TAYLQS7RAVA5CNFSM4JJLTDS2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLAHKZA#discussion_r344484634>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKQMTLVUBNYF22L45XUHGA3QS7RAVANCNFSM4JJLTDSQ>
.
|
It's good to migrate in a separate PR. Changes should be relevant to the issue at hand. Refactoring should be done in separate PRs to ensure no side effect bugs are introduced and git blame remains indicative of respective changes. |
Sure, Will make revert this changes 👍 |
@iamareebjamal Reverted the change |
Co-Authored-By: Areeb Jamal <jamal.areeb@gmail.com>
@iamareebjamal done, Please review. |
Fixes #3594