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

Related data mouseover #732

Merged
merged 37 commits into from
Dec 6, 2023
Merged

Related data mouseover #732

merged 37 commits into from
Dec 6, 2023

Conversation

ItIsJordan
Copy link
Collaborator

@ItIsJordan ItIsJordan commented Nov 23, 2023

Adds new functions to the HEPSubmission and DataSubmission  class to query for other related data object. Also adds a function to generate a dictionary generator function for combining recid/title data for the tooltip feature.
Updates the code to use the new related data functions.
Updates the related page link text to alter the record section to display record ID, with the title as a tooltip. Updates table DOI links to display the table name, with description as a tooltip.
Updates the testing for related record linking, currently working out an issue with results collection for checking.
Moves the title to the anchor tag for consistency with the other related data elements.
Fixes an error with the get_related_datasubmissions function in DataSubmission where the logic was incorrect. Also fixes the DataSubmission.get_table_data_list to use the correct relation get function.
Adds tests for the related and related_to_this functions for DataSubmission and HEPSubmission objects
Adds e2e tests to check the new tooltip update works for related data for records and tables.
Removes an unnecessary query in the get_table_data_list and get_record_data_list functions. Cleans up the variables, and removes a redundant check.
Adds comments to test functions, small cleaning changes to the new feature code.
Moves functions for related data from the models.py file and common.py into /records/api.py. Updates test_records.py to use new functions
Updates the related cleanup function to use the get_latest_hepsubmission function to ensure it always gets the most recent version, as prior versions may/will not have related data.
Updates a regular expression for checking url within a webpage to be more open in e2e/test_records.py:test_related_records
Adds a partially completed test for version tooltip testing in the e2e testing. Also adds the relevant testing data.
Adds finished test to check functionality of record page related table link tooltips.
Fiix broken path in related record test in test_records
Fixes path in test_records that breaks during full test run
Fixes docstring in newly added functions.
Updates some comments in branch changes
@coveralls
Copy link

coveralls commented Nov 23, 2023

Coverage Status

coverage: 83.168% (+0.1%) from 83.02%
when pulling dea20bd on related-data-mouseover
into 87cf880 on main.

Copy link
Member

@GraemeWatt GraemeWatt left a comment

Choose a reason for hiding this comment

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

Thanks, this is great. I left a few comments, mainly related to the subtleties of linking to records with multiple finished versions, which we discussed at our last meeting in my office.

hepdata/modules/records/api.py Outdated Show resolved Hide resolved
hepdata/modules/records/api.py Outdated Show resolved Hide resolved
hepdata/modules/records/api.py Outdated Show resolved Hide resolved
hepdata/modules/records/api.py Outdated Show resolved Hide resolved
hepdata/modules/records/assets/js/hepdata_tables.js Outdated Show resolved Hide resolved
Updates related data testing in test_records as expected rendering has changed.
Fixes directory paths in test_records.py related version testing.
Fixes comment for doi example in hepdata_tables.js
Removes unused import from api.py
Updates the related data functions in records/api.py to be request latest versions where possible
Updates pathing in related data testing
Makes the set used in get_related_to_this_hepsubmissions much simpler than before
Fixes the e2e related record test to dynamically generate the ids to ensure functionality when ran standalone and alongside the test suite
@GraemeWatt GraemeWatt requested a review from sjmf November 29, 2023 14:07
Copy link
Collaborator

@sjmf sjmf left a comment

Choose a reason for hiding this comment

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

A good PR. Have left a couple of comments to address in there, possibly with another commit.

hepdata/modules/records/api.py Outdated Show resolved Hide resolved
hepdata/modules/records/api.py Show resolved Hide resolved
hepdata/modules/records/api.py Outdated Show resolved Hide resolved
tests/submission_test.py Outdated Show resolved Hide resolved
tests/submission_test.py Show resolved Hide resolved
ItIsJordan and others added 4 commits November 29, 2023 16:11
Updates commenting in test_related_records in submission_test.py to clarify a change to the dummy data
Fixes indentation for consistency in the related data functions in records/api.py.
Cleans up some lengthy import statements in submission_test.py, and records/api.py
@GraemeWatt GraemeWatt merged commit b8837fa into main Dec 6, 2023
6 checks passed
@GraemeWatt GraemeWatt deleted the related-data-mouseover branch December 6, 2023 18:17
GraemeWatt added a commit to HEPData/hepdata-submission that referenced this pull request Dec 8, 2023
* Mention improvements introduced in HEPData/hepdata#732.
* Mention that hepdata_lib can be used to write new fields.
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.

records: allow users to preview content of bidirectional links before clicking
4 participants