Skip to content

Conversation

@YogeshSharma0201
Copy link
Contributor

@YogeshSharma0201 YogeshSharma0201 commented Jan 10, 2019

Checklist

  • 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)

Short description of what this resolves:

Event Import is working now and previous import jobs are being displayed.
Only zip files can be used for importing as of now and other importing methods are not implemented on backend.

Changes proposed in this pull request:

  • Add model for import jobs
  • Added neccessary actions and function to handle import

import

Fixes #767
Backend Changes PR: fossasia/open-event-server#5514

Copy link
Member

@rajvaibhavdubey rajvaibhavdubey left a comment

Choose a reason for hiding this comment

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

Fix Conflicts, Your branch is out of date.

@abhinavk96 abhinavk96 changed the title fix_import_event Fix Event Imports Jan 17, 2019
Copy link
Contributor

@abhinavk96 abhinavk96 left a comment

Choose a reason for hiding this comment

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

Hi @YogeshSharma0201 Any updates on this?

@YogeshSharma0201
Copy link
Contributor Author

@CosmicCoder96 i have removed jquery usage.

},
async model() {

const data = await this.get('store').findAll('importJob');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a valid query. There is no importJob model, please try importing the event yourself, and request a review when it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have implemented the importJob model both in backend and fronted. Please check fossasia/open-event-server#5514.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tested it locally, it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

@YogeshSharma0201 Oh okay will check on the server.

Copy link
Contributor

@abhinavk96 abhinavk96 left a comment

Choose a reason for hiding this comment

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

@YogeshSharma0201 Okay, so excellent work on this PR. You actually combined two separate issues into this single PR.

  • Issue A: One issue comprises of just getting an event to import successfully on the /v1/events/import/json endpoint.
  • Issue B: And then next was listing the import jobs for which you have created a new model on the server.
    We will be proceeding step by step. Please keep this PR and the PR on the server Open, I will add an appropriate label to this.
    Next, please send a new PR in which only issue A gets fixed. Issue A won't require any changes on the server I believe. If you have any confusion regarding these instructions please ping me on gitter, thanks !

@YogeshSharma0201
Copy link
Contributor Author

@CosmicCoder96 Issue A would also require changes on server side, see changes in file app/models/event.py and app/api/helpers/utilities.py in server side pr, these changes are required for successfully importing events. Should i still open a separate pr?

@abhinavk96
Copy link
Contributor

@YogeshSharma0201 Yes, please still open a separate PR. Also, we can't have the mentioned changes in the utilities.py, it will break several other things, try to find a workaround to that.

Copy link
Contributor

@abhinavk96 abhinavk96 left a comment

Choose a reason for hiding this comment

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

@YogeshSharma0201 You can now proceed with this. Update it please, thanks.

@kushthedude
Copy link
Member

@YogeshSharma0201 Still on it ?

@YogeshSharma0201
Copy link
Contributor Author

@kushthedude this PR is complete and updated (no merge conflicts)

@kushthedude
Copy link
Member

@YogeshSharma0201 thats great , @CosmicCoder96 review please.

@abhinavk96 abhinavk96 merged commit 90daa57 into fossasia:development May 15, 2019
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.

Implement event-import API to events/import

6 participants