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

fix: Never show AD col on drafts_for_ad.html #6914

Merged
merged 4 commits into from
Jan 12, 2024

Conversation

jennifer-richards
Copy link
Member

This should fix a transient HTML validation failure caused by assuming ad.name != doc.ad.plain_name() indicated that ad != doc.ad.

That check was, as far as I can tell, unnecessary because the search results were selected using doc.ad == ad. The check is removed here.

I'm leaving in the initial change from including ad_name to ad in the template context because I don't see a benefit to handing around the string instead of the full record.

For the record, the transient failure was:

======================================================================
FAIL: run_template_validation (ietf.utils.test_runner.TemplateValidationTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/datatracker/datatracker/ietf/utils/test_runner.py", line 343, in run_template_validation
    self.validate_html.validate(self)
  File "/__w/datatracker/datatracker/ietf/utils/test_runner.py", line 1039, in validate
    testcase.fail('\n'.join(errors))
AssertionError: 
/tmp/html-validate-9azr981j/222fdd5170477301/ietf/templates/doc/drafts_for_ad.html:
     </td>      </tr>      
          ^^^^^^^^^^^
          info: A table row was 6 columns wide and exceeded the column count established by the first row (5).
Source snippet, lines 789 to 800:
789:                 <i class="bi bi-envelope"></i></a></span>
790:             
791:             <br>
792:             
793:         </td>
794:     
795: </tr>
796:         
797:     </tbody>
798: 
799: </table>
800: 

Results being shown are already filtered by AD if the `ad`
variable is not `None`
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (287cf0f) 88.80% compared to head (f5fd9ac) 88.80%.
Report is 3 commits behind head on main.

❗ Current head f5fd9ac differs from pull request most recent head 1bce192. Consider uploading reports for the commit 1bce192 to get more accurate results

Files Patch % Lines
ietf/utils/markup_txt.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6914      +/-   ##
==========================================
- Coverage   88.80%   88.80%   -0.01%     
==========================================
  Files         284      285       +1     
  Lines       40361    40377      +16     
==========================================
+ Hits        35843    35857      +14     
- Misses       4518     4520       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rjsparks
rjsparks previously approved these changes Jan 12, 2024
@rjsparks rjsparks merged commit cb70da9 into ietf-tools:main Jan 12, 2024
7 checks passed
@jennifer-richards jennifer-richards deleted the ad-column-fix branch January 12, 2024 23:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants