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

Support "Add new instrument name"; Initialize "Upload image"; #164

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

Support "Add new instrument name"; Initialize "Upload image"; #164

wants to merge 13 commits into from
+1,107 −360

Conversation

kunfang98927
Copy link
Contributor

@kunfang98927 kunfang98927 commented Oct 11, 2024 •
edited
Loading

  • Support "add new instrument name" by a modal; New names can be submitted to UMIL and optionally to Wikidata;
  • Initialize "upload image" UI;
  • Remove "instrument detail" page;
  • Add "description" field into InstrumentName model;
  • "Alias" will be stored as instances of InstrumentName;
  • Fix some small bugs, such as incorrect French translation;
  • Add more percussions to the instrument list;

Resolves: #150
Resolves: #160
Resolves: #161
Resolves: #163
Resolves: #165

@kunfang98927
feat: initialize image upload UI
aa13e25 
- remove "instrument-detail" page
- add two buttons on each instrument card: "Upload new images" & "View on wikidata"
- initialize image upload modal UI

Refs: #150
@kunfang98927
fix: update "upload img" init & remove unused css
eaca2c1 
Refs: #150
@kunfang98927
feat: initialize image upload UI
4af1fee 
- remove "instrument-detail" page
- add two buttons on each instrument card: "Upload new images" & "View on wikidata"
- initialize image upload modal UI

Refs: #150
@kunfang98927
fix: update "upload img" init & remove unused css
e85db70 
Refs: #150
@kunfang98927
fix: add "description" & "alias" fields into InstrumentName model
6c04418 
- add "description" & "alias" fields into InstrumentName model;
- fix "Non classifiés" in facet search;
- support only English and French in facet search;
- fix a bug: replace `active_language.en_label == "french"` with `... "French"`;

Refs: #160 #161
@kunfang98927
Merge branch 'img-upload' of https://github.com/DDMAL/VIM into img-up…
b29e655 
…load
@kunfang98927
feat: add new instrument name
364ff92 
- add new instrument name, source, description, and alias by a modal;
- optionally publish to wikidata;

Refs: #163
@kunfang98927
fix: prettier fix
4539e3c 
Refs: #163
@kunfang98927
fix: update prettier version in pre-commit
f564ffb
@kunfang98927
fix: keep prettier version same as pre-commit
fe5801c
@kunfang98927
fix: update instrument list (add percussions)
888e84b 
Refs: #165
@kunfang98927
fix: refactor code for "add instrument name"
8079346 
- remove "alias" field from "InstrumentName" model;
- move "publish_name" to a separate .py file
- use "csrf_protect" decorator for "publish_name"
- extract the common code in "mansonryView.html" and "stdView.html"

Refs: #163
dchiller
dchiller reviewed Oct 22, 2024
View reviewed changes
web-app/django/VIM/apps/instruments/management/commands/import_instruments.py
@@ -15,7 +15,7 @@ class Command(BaseCommand):

NOTE: For now, this script only imports instrument names in English and French. It
also only imports a set of previously-curated instruments that have images available.
This list of instruments is stored in startup_data/vim_instruments_with_images-15sept.csv
This list of instruments is stored in startup_data/all_instruments_with_16aug_2024.csv
Copy link
Contributor

Choose a reason for hiding this comment

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

No longer the right file name. You could probably just take out this comment and make the file name a command-line argument or a constant.

@dchiller
Copy link
Contributor

I get the following when I try to publish a name:

image

dchiller
dchiller reviewed Oct 22, 2024
View reviewed changes
web-app/django/VIM/apps/instruments/views/publish_name.py
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm overall confused by the authentication process for wikidata? You are hardcoding a username and password in this view?

Here's the first documentation I find on authentication with wikidata's API: https://www.wikidata.org/wiki/Wikidata:REST_API/Authentication. Not saying it's necessarily the way to go but I'm wondering where this version came from?

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 previously referred to the documentation on logging in here: https://www.mediawiki.org/wiki/API:Login#Python
Let me take a closer look.

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 think using OAuth is essential, as recommended in the link you provided, to avoid hardcoding credentials. I’ve registered for OAuth and am currently awaiting approval. Based on feedback from last Saturday's project meeting, we also need to offer users a choice between using UMIL’s public account or authorizing their own Wikidata account. I'll create a new issue for this requirement. This pull request can be on hold until the user authentication issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dchiller dchiller dchiller left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@kunfang98927 @dchiller