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

Integrates sponsors API to sponsors route #580

Merged
merged 3 commits into from
Aug 3, 2017
Merged

Integrates sponsors API to sponsors route #580

merged 3 commits into from
Aug 3, 2017

Conversation

dilpreetsio
Copy link
Member

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Changes proposed in this pull request:

  • Integrates sponsors API to sponsors route
  • Modifies link-input for valid urls

Fixes #579

@dilpreetsio dilpreetsio self-assigned this Jul 30, 2017
@codecov
Copy link

codecov bot commented Jul 30, 2017 •
edited
Loading

Codecov Report

Merging #580 into development will decrease coverage by 0.53%.
The diff coverage is 7.4%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #580      +/-   ##
===============================================
- Coverage        49.24%   48.71%   -0.54%     
===============================================
  Files              297      297              
  Lines             1917     1938      +21     
===============================================
  Hits               944      944              
- Misses             973      994      +21
Impacted Files Coverage Δ
app/routes/events/view.js 33.33% <ø> (ø) ⬆️
app/mixins/event-wizard.js 66.66% <ø> (-8.34%) ⬇️
app/components/widgets/forms/link-input.js 28.57% <0%> (ø) ⬆️
app/components/forms/wizard/sponsors-step.js 13.33% <16.66%> (+4.24%) ⬆️
app/routes/events/view/edit/sponsors.js 4.76% <5%> (-28.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c0ce8b...54d7fa1. Read the comment docs.

@dilpreetsio
Copy link
Member Author

This will be merged after fossasia/open-event-server#4201 & fossasia/open-event-server#4203 are resolved on the server.

@dilpreetsio
Copy link
Member Author

@niranjan94 This is working now, as expected however there is one thing I needed to confirm before this merges, will we be deleting all the sponsors if the user turns the sponsors off?

@niranjan94
Copy link
Member

@geekyd yes. We will be.

@dilpreetsio
Copy link
Member Author

@niranjan94 Please review. Everything seems to work now. Thanks

@dilpreetsio dilpreetsio requested a review from a team August 1, 2017 15:12
@ghost
Copy link

ghost commented Aug 1, 2017

Hi @geekyd!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

@niranjan94
Copy link
Member

Deployment link: https://pr-580-open-event-frontend-fossasia.surge.sh

@ghost
Copy link

ghost commented Aug 2, 2017

Hi @geekyd!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

@niranjan94
Copy link
Member

Deployment link: https://pr-580-open-event-frontend-fossasia.surge.sh

@dilpreetsio dilpreetsio requested a review from niranjan94 August 2, 2017 06:34
niranjan94
niranjan94 suggested changes Aug 2, 2017
View reviewed changes
app/components/forms/wizard/sponsors-step.js Outdated
},

didInsertElement() {
if (this.get('data.event.sponsors') && !this.get('data.event.sponsors').toArray().length) {
Copy link
Member

Choose a reason for hiding this comment

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

data.event.sponsors.length will give you length. No need to convert to array.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@niranjan94
Copy link
Member

Deployment link: https://pr-580-open-event-frontend-fossasia.surge.sh

@abhinavk96 abhinavk96 merged commit f36229e into fossasia:development Aug 3, 2017
@ghost ghost added the ready-to-ship label Aug 3, 2017
@dilpreetsio dilpreetsio mentioned this pull request Aug 4, 2017
Merged
5 tasks
@iamareebjamal iamareebjamal mentioned this pull request Sep 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niranjan94 niranjan94 niranjan94 approved these changes

@abhinavk96 abhinavk96 abhinavk96 approved these changes

Assignees

@dilpreetsio dilpreetsio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement sponsors API to sponsors-step form
3 participants
@dilpreetsio @niranjan94 @abhinavk96